Skip to content
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

Jump to bottom

Deprecate alias 'thirsty' from all usages #61245

Merged
merged 4 commits into from Aug 28, 2019
Merged

Deprecate alias 'thirsty' from all usages #61245

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
817c403
Deprecate alias 'thirsty' from all usages
jillr Aug 23, 2019
3358651
Now with added version quoting
jillr Aug 23, 2019
cefcc74
Handle deprecated aliases in mod_utils
jillr Aug 23, 2019
15534bc
Make alias deprecation a subkey of the canonical arg, and not a separ…
jillr Aug 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/ansible/module_utils/basic.py
View file
Open in desktop
Expand Up @@ -1427,6 +1427,12 @@ def _handle_aliases(self, spec=None, param=None, option_prefix=''):
alias_results, self._legal_inputs = handle_aliases(spec, param, alias_warnings=alias_warnings)
for option, alias in alias_warnings:
self._warnings.append('Both option %s and its alias %s are set.' % (option_prefix + option, option_prefix + alias))
if 'deprecated_aliases' in spec.keys():
if spec['deprecated_aliases']['alias'] in param.keys():
self._deprecations.append(
{'msg': "Alias '%s' is deprecated. See the module docs for more information" % spec['deprecated_aliases']['alias'],
'version': spec['deprecated_aliases']['version']})

return alias_results

def _handle_no_log_values(self, spec=None, param=None):
Expand Down
1 change: 1 addition & 0 deletions lib/ansible/module_utils/urls.py
View file
Open in desktop
Expand Up @@ -1418,6 +1418,7 @@ def url_argument_spec():
force_basic_auth=dict(type='bool', default=False),
client_cert=dict(type='path'),
client_key=dict(type='path'),
deprecated_aliases=dict(alias='thirsty', version='2.13'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this makes more sense as a list or something since more than one alias may need deprecation and might need a different deprecation version. If it was just this arg spec it probably wouldn't matter, but since basic.py is handling this other people are going to want to utilize it and it seems like it should be flexible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm reconsidering if this is the right approach anyway. It needs to go into the docs for the sanity check, which could probably be hand-waved for one case but feels pretty hacky as a general solution. Need to see if I can come up with a way to make an internal arg or something like that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i suggest a list of dicts, each one containing 'name, why, version, alternative' as we do for options

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternatively a dict of dicts, with key being teh deprecated alias and the other 3 keys as subdicts

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bcoca So you're thinking more like the latest rev but with 2 extra keys to be used in the msg text?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was even thinking of making it top level in argspec and not in the type itself, but either way works, as long as each alias can have their own version/msg

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where would you consider to be the top level in argspec? If I missed a better location to do this in I'd be interested to look into it.

)


Expand Down
4 changes: 4 additions & 0 deletions lib/ansible/modules/files/copy.py
View file
Open in desktop
Expand Up @@ -60,6 +60,7 @@
- Influence whether the remote file must always be replaced.
- If C(yes), the remote file will be replaced when contents are different than the source.
- If C(no), the file will only be transferred if the destination does not exist.
- Alias C(thirsty) has been deprecated and will be removed in 2.13.
type: bool
default: yes
aliases: [ thirsty ]
Expand Down Expand Up @@ -510,6 +511,9 @@ def main():
supports_check_mode=True,
)

if module.params.get('thirsty'):
module.deprecate('The alias "thirsty" has been deprecated and will be removed, use "force" instead', version='2.13')

src = module.params['src']
b_src = to_bytes(src, errors='surrogate_or_strict')
dest = module.params['dest']
Expand Down
4 changes: 4 additions & 0 deletions lib/ansible/modules/files/iso_extract.py
View file
Open in desktop
Expand Up @@ -56,6 +56,7 @@
description:
- If C(yes), which will replace the remote file when contents are different than the source.
- If C(no), the file will only be extracted and copied if the destination does not already exist.
- Alias C(thirsty) has been deprecated and will be removed in 2.13.
type: bool
default: yes
aliases: [ thirsty ]
Expand Down Expand Up @@ -117,6 +118,9 @@ def main():
force = module.params['force']
executable = module.params['executable']

if module.params.get('thirsty'):
module.deprecate('The alias "thirsty" has been deprecated and will be removed, use "force" instead', version='2.13')

result = dict(
changed=False,
dest=dest,
Expand Down
4 changes: 4 additions & 0 deletions lib/ansible/modules/net_tools/basics/get_url.py
View file
Open in desktop
Expand Up @@ -61,6 +61,7 @@
will only be downloaded if the destination does not exist. Generally
should be C(yes) only for small local files.
- Prior to 0.6, this module behaved as if C(yes) was the default.
- Alias C(thirsty) has been deprecated and will be removed in 2.13.
type: bool
default: no
aliases: [ thirsty ]
Expand Down Expand Up @@ -446,6 +447,9 @@ def main():
mutually_exclusive=[['checksum', 'sha256sum']],
)

if module.params.get('thirsty'):
module.deprecate('The alias "thirsty" has been deprecated and will be removed, use "force" instead', version='2.13')

url = module.params['url']
dest = module.params['dest']
backup = module.params['backup']
Expand Down
4 changes: 4 additions & 0 deletions lib/ansible/modules/net_tools/basics/uri.py
View file
Open in desktop
Expand Up @@ -154,6 +154,7 @@
force:
description:
- If C(yes) do not get a cached copy.
- Alias C(thirsty) has been deprecated and will be removed in 2.13.
type: bool
default: no
aliases: [ thirsty ]
Expand Down Expand Up @@ -574,6 +575,9 @@ def main():
mutually_exclusive=[['body', 'src']],
)

if module.params.get('thirsty'):
module.deprecate('The alias "thirsty" has been deprecated and will be removed, use "force" instead', version='2.13')

url = module.params['url']
body = module.params['body']
body_format = module.params['body_format'].lower()
Expand Down
1 change: 1 addition & 0 deletions lib/ansible/plugins/doc_fragments/url.py
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@ class ModuleDocFragment(object):
force:
description:
- If C(yes) do not get a cached copy.
- Alias C(thirsty) has been deprecated and will be removed in 2.13.
type: bool
default: no
aliases: [ thirsty ]
Expand Down