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

Allow to deprecate options and aliases by date #68177

Merged
merged 28 commits into from
May 27, 2020

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Allows to deprecate options and aliases also by date and not only by version.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lib/ansible/module_utils/basic.py
lib/ansible/module_utils/common/parameters.py
lib/ansible/module_utils/common/warnings.py
lib/ansible/module_utils/csharp/Ansible.Basic.cs
test/lib/ansible_test/_data/sanity/pylint/plugins/deprecated.py
test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py
test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/schema.py

@felixfontein felixfontein changed the base branch from devel to temp-2.10-devel March 11, 2020 20:41
@ansibot
Copy link
Contributor

ansibot commented Mar 11, 2020

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.10 This issue/PR affects Ansible v2.10 backport This PR does not target the devel branch. feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. windows Windows community support:community This issue/PR relates to code supported by the Ansible community. labels Mar 11, 2020
@felixfontein felixfontein changed the title [WIP] Allow to deprecate options and aliases by date Allow to deprecate options and aliases by date Mar 12, 2020
@felixfontein
Copy link
Contributor Author

(The remaining CI failures are package download timeouts, i.e. unrelated :) )

ready_for_review

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. labels Mar 12, 2020
@felixfontein
Copy link
Contributor Author

felixfontein commented Mar 17, 2020

TODO next:

@ansibot ansibot added has_issue needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 17, 2020
@felixfontein felixfontein changed the base branch from temp-2.10-devel to devel March 23, 2020 16:19
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 23, 2020
@mattclay
Copy link
Member

@felixfontein What's the intended workflow for this feature? Deprecation by version seems obvious to me, but I'm not immediately coming up with a scenario for deprecation by date.

@felixfontein
Copy link
Contributor Author

(Rebased so that the branch includes the changes from the routing PR, and changed behavior of Display.deprecated() if both version and date are specified: prefer date in that case. This behavior is now the same as in #69712.)

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels May 26, 2020
@nitzmahone
Copy link
Member

rebuild_merge

Comment on lines +94 to +95
if not re.match('^[0-9]{4}-[0-9]{2}-[0-9]{2}$', value):
raise ValueError(msg)
Copy link
Member

Choose a reason for hiding this comment

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

This logic appears to be redundant. The exception handler below should catch the same cases the regex does and return the same exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code below also accepts 2020-1-1, which is not ISO 8601. It does catch invalid dates such as 2019-02-29 though, which this regex won't catch.

Comment on lines +208 to +209
if not re.match('^[0-9]{4}-[0-9]{2}-[0-9]{2}$', v):
raise ValueError(msg)
Copy link
Member

Choose a reason for hiding this comment

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

Same as previous comment about redundant logic.

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 27, 2020
@ansibot ansibot merged commit ea04e00 into ansible:devel May 27, 2020
@felixfontein felixfontein deleted the deprecate-by-date branch May 27, 2020 05:43
@felixfontein
Copy link
Contributor Author

@mattclay @bcoca @sivel @jborean93 @nitzmahone thanks a lot for reviewing and merging! :)

felixfontein added a commit to felixfontein/ansible that referenced this pull request May 27, 2020
felixfontein added a commit to felixfontein/ansible that referenced this pull request May 27, 2020
nitzmahone pushed a commit that referenced this pull request May 28, 2020
…make validate-modules ensure version and date match (#69727)

* Allow to deprecate module by date in documentation.

* Make sure deprecation date/version match between module docs and meta/runtime.yml.

* Unrelated fix: don't compare deprecated module version to Ansible's version in collection.

* Allow documentation's removal version to be something else than fixed list of Ansible versions for collections.

* Linting.

* Allow to deprecate plugin options by date.

* Add changelog fragment for deprecation by date (also covers #68177).
@ansible ansible locked and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.10 This issue/PR affects Ansible v2.10 backport This PR does not target the devel branch. docs This issue/PR relates to or includes documentation. docsite This issue/PR relates to the documentation website. feature This issue/PR relates to a feature request. has_issue shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. windows Windows community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants