Add dependent collections to galaxy.yml and requirements.yml #2145
Conversation
|
Build failed. ✔️ ansible-galaxy-importer SUCCESS in 7m 59s |
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 20m 31s |
GomathiselviS
changed the title
| @@ -93,6 +93,7 @@ | |||
| # Note: These examples do not set authentication details, see the AWS Guide for details. | |||
|
|
|||
| # Basic creation example: | |||
| # Testing | |||
There was a problem hiding this comment.
I guess this can be removed.
| @@ -9,6 +9,14 @@ description: A variety of Ansible content to help automate the management of AWS | |||
| license_file: COPYING | |||
| tags: [amazon, aws, cloud] | |||
| repository: https://github.com/ansible-collections/amazon.aws | |||
| dependencies: | |||
| "ansible.windows": "*" | |||
There was a problem hiding this comment.
These dependencies are only needed for integration tests. Is there a way to add them only in the file tests/integration/requirements.yml and use this file to pull the dependencies on zuul?
There was a problem hiding this comment.
This should not appear here, the collection does not have any dependencies with these collections.
This is needed for test purposes only.
There was a problem hiding this comment.
From my understanding, collections specified in tests/integration/requirements.yml are not installed by Zuul. If we remove these collections from the required-projects list in Zuul, we need to ensure they are added here to ensure the tests pass on Zuul.
There was a problem hiding this comment.
We have relied on the required-projects list to install the collections used in our tests. Going forward, there's a possibility that the ansible.netcommon project might be removed from Zuul, as its tests no longer utilize Zuul for CI.
There was a problem hiding this comment.
From my understanding, collections specified in tests/integration/requirements.yml are not installed by Zuul.
Then, IMO, before we update the Zuul configs, that's what we should be fixing. There is a need to separate the run time dependencies from the test dependencies. Just because we install something to make our lives easier when building our test cases shouldn't mean we force customers to install them for run-time it's bad practice.
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 25s |
| @@ -9,6 +9,14 @@ description: A variety of Ansible content to help automate the management of AWS | |||
| license_file: COPYING | |||
| tags: [amazon, aws, cloud] | |||
| repository: https://github.com/ansible-collections/amazon.aws | |||
| dependencies: | |||
| "ansible.windows": "*" | |||
There was a problem hiding this comment.
This should not appear here, the collection does not have any dependencies with these collections.
This is needed for test purposes only.
| @@ -3,3 +3,7 @@ collections: | |||
| - ansible.windows | |||
| - ansible.utils # ipv6 filter | |||
| - amazon.cloud # used by integration tests - rds_cluster_modify | |||
| - ansible.netcommon | |||
There was a problem hiding this comment.
to be consistent with the Zuul configuration, the dependency should be defined using:
- name: https://github.com/ansible-collections/{{ namespace }}.{{ name }}.git
type: git
version: main
| "amazon.cloud": "*" | ||
| "ansible.netcommon": "*" | ||
| "community.crypto": "*" | ||
| "community.aws": "*" |
There was a problem hiding this comment.
| "community.aws": "*" |
In theory amazon.aws can be used on its own
| @@ -3,3 +3,7 @@ collections: | |||
| - ansible.windows | |||
| - ansible.utils # ipv6 filter | |||
| - amazon.cloud # used by integration tests - rds_cluster_modify | |||
| - ansible.netcommon | |||
| - community.crypto | |||
| - community.aws | |||
There was a problem hiding this comment.
For testing amazon.aws (and for testing community.aws, amazon.aws) this needs to come directly from git, not from galaxy. Otherwise we can't coordinate breaking changes between the two repos.
There was a problem hiding this comment.
@GomathiselviS community.aws should not be defined here, it should be kept in the required project on the integration job because we are testing both collections with the same branch (for instance stable-X on will be tested with community.aws:Stable-X)
|
I think the consensus here is that this change shouldn't be merged as is, these collections aren't needed for |
SUMMARY
Depends-On: ansible/ansible-zuul-jobs#1878
This PR adds dependent collections (needed for integration tests) to galaxy.yml and tests/integration/requirements.yml.
These collections can be removed from required-projects used in ansible-zuul-jobs.
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION