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

Collection Requirements Violation - Semantic Versioning #217

Closed
Tracked by #223
felixfontein opened this issue May 23, 2023 · 7 comments · Fixed by #244
Closed
Tracked by #223

Collection Requirements Violation - Semantic Versioning #217

felixfontein opened this issue May 23, 2023 · 7 comments · Fixed by #244

Comments

@felixfontein
Copy link

According to the changelog (https://github.com/ansible-collections/hetzner.hcloud/blob/main/CHANGELOG.rst#breaking-changes-porting-guide), the minor 1.12.0 release contains a breaking change. This is explicitly prohibited by semantic versioning (https://semver.org/) - breaking changes must happen in new major versions.

Semantic versioning is one of the main collection requirements for inclusion in Ansible (https://docs.ansible.com/ansible/devel/community/collection_contributors/collection_requirements.html#versioning-and-deprecation).

In today's Ansible 7.6.0 release, we pinned hetzner.hcloud to 1.11.0 to avoid breaking changes.

Please undo this breaking change and create a new 1.13.0 minor release without it as soon as possible, or risk freezing hetzner.hcloud to 1.11.0 for the remainder of the Ansible 7 and 8 release cycles and removal of the collection from Ansible 9.

@apricote
Copy link
Collaborator

Hey @felixfontein,

we have always required an up-to-date version of hcloud-python for use with the ansible collection. When we add a new field to the API, we add that to hcloud-python and then to the ansible collection. This was not well communicated in the past, so I decided to add a disclaimer to this versions release. It is unfortunate that Ansible collections can not depend on Pypi packages.

Do you have an alternative suggestion for how we can implement a dependency on hcloud-python sensibly? I see these options, but do not know which are common in the ansible ecosystem:

  • Bump major version for every update to our hcloud-python version
  • Implement new features in a backwards compatible way
    • This would require us to guard every single field access from hcloud-python behind a version check
  • Vendor/bundle hcloud-python with the released code for the ansible collection
    • Not sure how this would handle dependencies of hcloud-python
    • We can then always include an up-to-date version of hcloud-python with new collection releases and the user would never need to care about the underlying hcloud-python version

@felixfontein
Copy link
Author

Do you have an alternative suggestion for how we can implement a dependency on hcloud-python sensibly?

This is a tricky problem. It usually gets worse when collections work with services that can also be self-hosted, so there are both multiple versions of the library to think about, and multiple versions of the service itself. You are kind of lucky that you "only" have multiple versions of the library ;)

  • Bump major version for every update to our hcloud-python version

I think this would be the worst solution. (Assuming that hcloud-python gets updated more than 1-2 per year.)

  • Implement new features in a backwards compatible way

    * This would require us to guard **every single field access** from `hcloud-python` behind a version check
    

That's what some collections do (especially the ones dealing with self-hosted services). Usually you can implement some infrastructure code which makes it easy to do this without manual extra work. But this is definitely some work and not so easy to introduce into an existing codebase.

  • Vendor/bundle hcloud-python with the released code for the ansible collection

This is probably the best solution.

  * Not sure how this would handle dependencies of `hcloud-python`

I guess the same way as you are handling hcloud-python itself right now: you have to tell the users to install these. Right now there seems to be two: requests and python-dateutil. Asking users to manually install these should be fine.

BTW, you only use python-dateutil in the main code (outside the tests) for the dateutil.parser.isoparse function. Python 3.7+ (which happens to be the minimum Python version hcloud-python requires) has datetime.datetime.fromisoformat. While it is more limited than the python-dateutil function (at least for Python <= 3.10), it might be enough for your use-cases. If it is enough, I would suggest to use that one and drop the dependency on python-dateutil. Then you only depend on requests, which is a very common dependency.

@apricote
Copy link
Collaborator

apricote commented Jun 2, 2023

Thanks a lot for the guidance @felixfontein! I opened #218 to track the vendoring

@apricote
Copy link
Collaborator

@resmo I will respond here instead of at #218 (comment), as this is where the previous discussion was held. You said:

I am not sure if I understand this correctly but I would not bundle it but test for the hcloud-python version in the code. This is what other collections do as well.

I am not quite sure what you meant, so I am just going to respond to both intepretations:

Read the version and show a warning (similar to #211)

This way we would still effectivly require newer version of hcloud-python with each release of the ansible collection. This goes against the "no breaking changes in minor versions" rule that we have been breaking so far and were asked to stop breaking.

Gate every access to new fields behind a version check

We would need to write a wrapper function for accessing any methods & fields from hcloud-python objects. This requires some refactoring and adds significant complexity to the implementation of the collection.

@jooola
Copy link
Collaborator

jooola commented Jul 11, 2023

To solve this issue, I am going to cherry-pick #244 into the stable-1 branch and release a new 1.x version. This should remove the pin on the hetzner.hcloud collection.

@jooola
Copy link
Collaborator

jooola commented Jul 14, 2023

@felixfontein I just released 1.16.0 which includes the hcloud package vendoring change.

The issue behind the breaking semantic versioning is now solved. I hope this allows you to unpin the collection from the ansible package ? Thanks for your help.

Closing this as resolved, feel free to reopen/comment if something is still required.

@felixfontein
Copy link
Author

Thanks! I've created a PR (ansible-community/ansible-build-data#267) to un-pin hetzner.hcloud in Ansible.

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

Successfully merging a pull request may close this issue.

3 participants