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

NWO: All version numbers should be tagged (automatically or manually) #178

Closed
felixfontein opened this issue May 21, 2020 · 7 comments
Closed

Comments

@felixfontein
Copy link

felixfontein commented May 21, 2020

Proposal: All version numbers should be tagged

Author: Felix fontein <@felixfontein>

Date: 2020-05-21

Motivation

In the NWO, both ansible-base (ansible.builtin) and collections have version numbers. If a feature is added, deprecated, or removed, it should be clear in which version of which collection this happen(s/ed).

Example 1: module test in collection foo.bar uses files doc fragment. Option attributes (from files) says version_added: 2.3, while foo.bar has version 1.0.0. Users reading this in the docs are confused.

Example 2: doc fragment in module_utils in foo.bar deprecates an existing option baz, to be removed in version 2.0.0 of foo.bar. The module stuff from foo.fancy uses the doc fragment and module_utils from foo.bar. When a user runs foo.facy.stuff with this option, a deprecation warning This option will be removed in version 2.0.0 will be printed. Since foo.fancy has version 2.5.1, the user is confused.

Problems

Make it clear to users in documentation and deprecation warnings which collection is talked about. The user does not know that option attributes of foo.bar.test comes from ansible.builtin, or that the option in foo.fancy.stuff is deprecated because it comes from the foo.bar collection.

A more detailled writeup: https://github.com/ansible/community/wiki/Version-numbers-for-documentation-deprecation-removal-in-the-collection-world

Solution proposal

  • All versions shown to the user should either belong to the current collection where the module/plugin/... belongs to, or should indicate which collection the version number belongs to.
  • The condensed output (ansible-doc) would be ansible.builtin:2.10 or community.general:1.3.0; the HTML docs output could be Added in Ansible-base 2.10 or Added in community.general 1.3.0.
  • For version_added, the collection name can be tracked automatically: version_added is added in plugin or module docs, and in doc fragments. Since Ansible knows where each fragment is loaded from, the loading code can tag all version_added values.
new_option:
    description: '...'
    version_added: 1.3.0  # no need to tag, Ansible knows this is in collection foo.bar
  • Deprecations for plugin options are done in documentation and doc fragments as well. These should also be tagged automatically.
  • All deprecation version numbers (except plugin/module deprecation) that are handled in code - that is display.deprecated() calls, module.deprecate() calls, removed_in_version and deprecated_aliases.version in module argument spec - must be passed explicitly tagged version numbers:
argument_spec = dict(
    old_option=dict(type='str', removed_in_version='foo.bar:2.0.0'),
    option_with_old_alias(type='str', aliases=['optionwitholdalias'],
                          deprecated_aliases=[dict(name='optionwitholdalias', version='foo.bar:3.0.0')]),
)
module.deprecate('Passing rabbits is deprecated', version='foo.baz:4.0.0')
display.deprecated('This will stop working', version='ansible.builtin:2.13')
  • This labelling in code has to be done in ansible-base (ansible.builtin) and in all collections.

Testing (optional)

ansible-test can validate some of these:

  1. In validate-modules, it can ensure that version numbers use the correct format if they appear in the documentation or in the module argument spec. I.e. it can check whether the tag is there, and when the tag is there, make sure that collection versions follow the semantic versioning version number definition, and for Ansible versions that they are parsable by StrictVersion.
  2. A pylint plugin can check all module.deprecate() and display.deprecation() calls, and ensure that they add the correct tags and that the verison number is correct (assuming that the version is passed as a literal in the call).

Documentation (optional)

This requires updating the dev guide, so that people know how to tag version numbers, and it should be announced in ansible/community#346 and ansible-collections/overview#45 so that module/plugin and collection maintainers know about this.

Anything else?

Q: Why not label all version numbers (i.e. also the ones appearing in documentation and doc fragments)?
A: For two reasons:

  1. Most version numbers which are used are version_added. Deprecations do not happen very often. So the task that happens most (adding version_added) is not complicated unnecessarily, while the task that is made more complicated doesn't happen very often.
  2. I would prefer all versions to be tagged automatically, but unfortunately for deprecation done by code this is not feasible. So let's automatically tag as much as possible.

Q: Isn't this a lot of work (to tag all version numbers in code)?
A: I think it is not that much work:

  1. Deprecation doesn't happen that often.
  2. Currently existing deprecations outside ansible-base have to be adjusted anyway, since these currently in most cases still use Ansible version numbers (due to migration). If this proposal is through before everyone updated these version numbers, it will be almost no extra work.
  3. I'm volunteering to do the work for community.general and community.network. That's probably the largest batch. :)

Q: Isn't this making forking collections (copying them and releasing them under a new name) more complicated?
A: Not much: right now you already have to adjust many places where the collection name is imprinted, and usually a global search-and-replace with some manual post-processing (verify all changes) does the job fine. Since deprecations don't happen too often, this should not increase the amount of work by much.

@felixfontein
Copy link
Author

I've updated the PR (ansible/ansible#69680) to also require that removed_at_date and deprecated_aliaes.date are tagged (these are also generated by code). These date-based deprecations have been added in ansible/ansible#68177.

@felixfontein
Copy link
Author

ansible/ansible#69680 has been merged, so this now works.

@AlanCoding
Copy link
Member

Was the intention for the version_added to allow for specifying the collection? For example:

    ask_inventory:
      description:
        - Prompt user for inventory on launch.
      type: bool
      version_added: "ansible.builtin:2.9"

The collection didn't introduce the option, it was carried over from Ansible 2.9. Likewise, if someone did fork a collection, the version numbers would be wrong unless the new collection backdated version or something, which would be meaningless anyway.

Right now this gets the invalid semantic version error, saying it got awx.awx:ansible.builtin:2.9, so it appends the collection name onto the front.

@felixfontein
Copy link
Author

No, that's not intended. version_added always refers to the collection it is part of. In this case, version_added should not be there, because the option was there when the module was introduced to the collection.

(During migration all version_added's were removed anyway, unfortunately even the ones with 2.10 as the value, which IMO should be there so the ACD docs will be more helpful.)

@felixfontein
Copy link
Author

ansible/ansible#69926 has been merged, which changes the implementation away from tagged version numbes/dates to extra fields for the collection name.

@felixfontein
Copy link
Author

@AlanCoding with the current implementation (now in 2.10 beta 1, so it shouldn't change again) you can explicitly specify the collection name. You might need to add ignore.txt entries in some cases, though.

@felixfontein
Copy link
Author

Closing this proposal, since a better system has been now implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants