-
Notifications
You must be signed in to change notification settings - Fork 23.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve ansible-galaxy handling of role versions #12904
Conversation
0009a8e
to
780019c
Compare
780019c
to
4f0023f
Compare
""" | ||
if self.version: | ||
return "%s (%s)" % (self.name, self.version) | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imho, this else is not usefull ?.
if self.version:
return xxxx
return xxxx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lujeni meh, it's really not that important. I prefer it the way it is, and you're entitled to your preference.
Unless you can point to actual documentation backing up your preferences though (PEP-8, Ansible coding standards, etc.), I'm unlikely to change my code.
4f0023f
to
a2b7c7a
Compare
+1 |
This is helpful towards solving both #11266 and ansible/galaxy-issues#49 |
@@ -318,3 +318,14 @@ def spec(self): | |||
} | |||
""" | |||
return dict(scm=self.scm, src=self.src, version=self.version, name=self.name) | |||
|
|||
@property | |||
def version_string(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not really version_string .. full_name? name_and_version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be __repr__
rather than version_string
:)
a2b7c7a
to
503a1cd
Compare
@bcoca should be ready for re-review now, I believe I've addressed your concerns. |
Ensure that role versions are considered when deciding whether or not to (re-)install a role. Issue a warning when the version of a dependency conflicts with the version of an already installed role Display what version of a role is being installed Show the versions when upgrading/downgrading a role. Implements ansible#11266
Ensure that force is required to change role versions
503a1cd
to
4945f41
Compare
👋 apologies for the bump, but this PR deserves some attention as it implements a much requested feature. |
Ooh, it's merged! |
In current stable (2.2), ansible galaxy install --force do erase a role, even if the version is not set. This commit should restore that specific behavior, in accordance to people reports: ansible#11266 (comment) It was also the behavior planned in the initial discussion: "if you're not fixing versions in your roles file, then it's fine to expect that the role will be reinstalled each time you run ansible-galaxy install.", cf ansible#12904
In current stable (2.2), ansible galaxy install --force do erase a role, even if the version is not set. This commit should restore that specific behavior, in accordance to people reports: #11266 (comment) It was also the behavior planned in the initial discussion: "if you're not fixing versions in your roles file, then it's fine to expect that the role will be reinstalled each time you run ansible-galaxy install.", cf #12904
In current stable (2.2), ansible galaxy install --force do erase a role, even if the version is not set. This commit should restore that specific behavior, in accordance to people reports: #11266 (comment) It was also the behavior planned in the initial discussion: "if you're not fixing versions in your roles file, then it's fine to expect that the role will be reinstalled each time you run ansible-galaxy install.", cf #12904 (cherry picked from commit 78836ec)
Ensure that role versions are considered when deciding
whether or not to (re-)install a role.
Issue a warning when the version of a dependency conflicts
with the version of an already installed role
Display what version of a role is being installed
Show the versions when upgrading/downgrading a role.
Implements #11266
Note that the behaviour with galaxy roles with an unspecified version is a little odd. This is because
meta/.galaxy_install_info
gets told the version of the role that actually gets installed when installing under galaxy. However, if you're not fixing versions in your rolesfile, then it's fine to expect that the role will be reinstalled each time you run ansible-galaxy install. (This is similar to the behaviour ofyum state=latest
, for example). One fix for this would be for.galaxy_install_info
to be told the version of the role specified, rather than the version of the role installed.