DNM: Create dist if it doesn't exist - #558
Conversation
Bumps Ansible pulp collections (pulp squeezer and our downstream ansible-collection-pulp) to the same versions we currently use in SKC. These should come with bugfixes and performance improvements. These now depend on pulp-glue and pulp-glue-deb pip packages, which have been added to requirements.txt
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughUpdates Python setup and dependencies, including newer Pulp collections and WalkthroughThe setup now uses Python 3.12.x and installs updated Pulp packages and collections. Debian and RPM repository tests now select existing distributions, create missing distributions, and re-query them. ChangesPulp tooling setup
Distribution query flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AnsibleTest as Ansible test
participant DistributionQuery as Distribution query
participant DistributionCreation as Distribution creation
AnsibleTest->>DistributionQuery: collect matching distributions
DistributionQuery-->>AnsibleTest: return distribution list
AnsibleTest->>DistributionCreation: create distribution when none exists
DistributionCreation-->>AnsibleTest: return created distribution
AnsibleTest->>DistributionQuery: re-query distributions
DistributionQuery-->>AnsibleTest: return updated distribution list
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7b9d3d0e-65eb-4758-a9c9-3bea5d7ba0a3
📒 Files selected for processing (4)
.github/actions/setup/action.ymlansible/test-pulp-repo-version-query.ymlrequirements.txtrequirements.yml
423a72a to
ef228b2
Compare
9ca7402 to
ea54d7d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 59d21ccc-ade3-4c9d-b095-01278aca2dcf
📒 Files selected for processing (1)
ansible/test-pulp-repo-version-query.yml
| test_pulp_repository_deb_repo_details: "{{ (_deb_composite | from_json).details }}" | ||
| vars: | ||
| _deb_composite: >- | ||
| {%- set short_map = test_pulp_repository_deb_repo_versions | default({}) -%} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Trace prior assignments and repeated imports of this fact producer.
rg -n -g '*.yml' -g '*.yaml' \
'test_pulp_repository_(deb|rpm)_repo_versions|test-pulp-repo-version-query\.yml|import_playbook:|include_tasks:' .Repository: stackhpc/stackhpc-release-train
Length of output: 5012
🏁 Script executed:
wc -l ./ansible/test-pulp-repo-version-query.ymlRepository: stackhpc/stackhpc-release-train
Length of output: 216
🏁 Script executed:
cat -n ./ansible/test-pulp-repo-version-query.yml | sed -n '115,165p'Repository: stackhpc/stackhpc-release-train
Length of output: 3457
🏁 Script executed:
cat -n ./ansible/test-pulp-repo-version-query.yml | sed -n '210,255p'Repository: stackhpc/stackhpc-release-train
Length of output: 2632
Initialise the short_map variables to {} to prevent accumulation across repeated executions.
The current implementation seeds short_map from the previous execution's fact using the default() filter, whilst details_map is reset to {}. When the playbook executes again, repositories that no longer exist in the current dev_pulp_distribution_deb or dev_pulp_distribution_rpm lists will persist in short_map, creating a mismatch with details_map.
The loop updates short_map entries only if a matching distribution exists; it does not remove entries that are no longer current. Downstream code that uses both test_pulp_repository_deb_repo_versions and test_pulp_repository_deb_repo_details facts may fail if an orphaned version key has no matching detail entry.
To fix:
- ansible/test-pulp-repo-version-query.yml, line 129: change
{%- set short_map = test_pulp_repository_deb_repo_versions | default({}) -%}to{%- set short_map = {} -%} - ansible/test-pulp-repo-version-query.yml, line 220: change
{%- set short_map = test_pulp_repository_rpm_repo_versions | default({}) -%}to{%- set short_map = {} -%}
📍 Affects 1 file
ansible/test-pulp-repo-version-query.yml#L129-L129(this comment)ansible/test-pulp-repo-version-query.yml#L220-L220
fb22840 to
949a4bb
Compare
949a4bb to
6dc9b09
Compare
This PR is only up for discussion, not to be merged