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

Remove ansible.posix dependency #1157

Conversation

felixfontein
Copy link
Collaborator

SUMMARY

As mentioned here by vendoring the one file used by the filesystem module.

(Also part of #354.)

ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

filesystem
collection

@ansibullbot ansibullbot added affects_2.10 bug This issue/PR relates to a bug community_review module module module_utils module_utils needs_triage new_plugin New plugin plugins plugin (any type) system tests tests labels Oct 22, 2020
@felixfontein
Copy link
Collaborator Author

I've also changed the shippable script to only install the requirements needed for every test type. This prevents collections to be installed for sanity checks which shoud only be needed during unit/integration tests.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added the unit tests/unit label Oct 22, 2020
tests/utils/shippable/shippable.sh Show resolved Hide resolved
tests/utils/shippable/shippable.sh Show resolved Hide resolved
@@ -0,0 +1,90 @@
# This code is part of Ansible, but is an independent component.
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we ensure that this file is kept in sync with ansible.posix? to prevent plugins/modules/system/aix_filesystem.py being exposed to a bug.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A very good question. Maybe we could add an extra sanity test which compares whether the files are equal. Then we'll quickly find out when they update it. (Disadvantage: it will fail for every PR. But then it'll surface more quickly...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gundalow @Andersson007 as a short-term solution, how about creating an issue for this, and maybe have a column in a project where issues tracking similar problems are tracked? (We could also use a label, but that doesn't work well cross-repository.)

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good to me

Co-authored-by: John R Barker <john@johnrbarker.com>
@gundalow
Copy link
Contributor

Yes to using major_change
Creating a tracking bug for keeping files in sync sounds good.

@felixfontein
Copy link
Collaborator Author

TODO:

(Also create and reference issues for other copies, like ipaddress in community.general and community.crypto.)

@felixfontein felixfontein merged commit 20f470c into ansible-collections:main Oct 28, 2020
@felixfontein felixfontein deleted the drop-ansible.posix-dependency branch October 28, 2020 11:57
@felixfontein
Copy link
Collaborator Author

@gundalow @Andersson007 thanks a lot for reviewing this!

patchback bot pushed a commit that referenced this pull request Oct 28, 2020
* Vendor plugins/module_utils/mount.py from ansible.posix, and drop ansible.posix dependency (except for testing).

* Add ignore.txt entries.

* Install test requirements conditionally.

* Apply suggestions from code review

Co-authored-by: John R Barker <john@johnrbarker.com>

* Bump to major changes.

Co-authored-by: John R Barker <john@johnrbarker.com>
(cherry picked from commit 20f470c)
felixfontein added a commit that referenced this pull request Oct 28, 2020
* Vendor plugins/module_utils/mount.py from ansible.posix, and drop ansible.posix dependency (except for testing).

* Add ignore.txt entries.

* Install test requirements conditionally.

* Apply suggestions from code review

Co-authored-by: John R Barker <john@johnrbarker.com>

* Bump to major changes.

Co-authored-by: John R Barker <john@johnrbarker.com>
(cherry picked from commit 20f470c)

Co-authored-by: Felix Fontein <felix@fontein.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug community_review module_utils module_utils module module new_plugin New plugin plugins plugin (any type) system tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants