Skip to content

[AAP-73775] Add Hub to DAB consumers#986

Open
jerabekjiri wants to merge 14 commits intoansible:develfrom
jerabekjiri:add-hub-to-dab-consumers
Open

[AAP-73775] Add Hub to DAB consumers#986
jerabekjiri wants to merge 14 commits intoansible:develfrom
jerabekjiri:add-hub-to-dab-consumers

Conversation

@jerabekjiri
Copy link
Copy Markdown
Contributor

@jerabekjiri jerabekjiri commented Apr 27, 2026

Description

Issue: https://redhat.atlassian.net/browse/AAP-73775

DAB consumers are missing Hub. This adds Hub to the list and runs unit test with current DAB.
How it works

  1. Checks out galaxy_ng into hub/ directory
  2. Runs tox -e py312 from the hub/ directory with DJANGO_ANSIBLE_BASE_BRANCH: ${{ github.sha }} - latest commit on the branch
  3. galaxy_ng's setup.py reads the DJANGO_ANSIBLE_BASE_BRANCH env var to determine which DAB commit to
    install:
django_ansible_base_branch = os.getenv(
    'DJANGO_ANSIBLE_BASE_BRANCH', '<pinned-commit>'
)
  1. By passing the current PR's commit SHA, pip install -e . (galaxy_ng) fetches and installs this PR's
    version of DAB directly instead of the default pinned commit
  2. Hub's unit tests (galaxy_ng.tests.unit) then run against this DAB version, catching any breaking
    changes before they're merged
  $GITHUB_WORKSPACE/
  └── hub/                    ← galaxy_ng
      └── tox.ini             ← pip install -e . uses DJANGO_ANSIBLE_BASE_BRANCH=${{ github.sha }}

Requires https://github.com/ansible-automation-platform/galaxy_ng/pull/474 (done)

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Test update
  • Refactoring (no functional changes)
  • Development environment change
  • Configuration change

Self-Review Checklist

  • I have performed a self-review of my code
  • I have added relevant comments to complex code sections
  • I have updated documentation where needed
  • I have considered the security impact of these changes
  • I have considered performance implications
  • I have thought about error handling and edge cases
  • I have tested the changes in my local environment

Summary by CodeRabbit

  • Chores
    • Added an automated CI job to run the Hub unit test suite using Python 3.12, ensuring tests execute against the current commit to improve reliability and catch regressions earlier.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 27, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new GitHub Actions job hub to .github/workflows/dab-consumers.yml that checks out ansible/galaxy_ng, installs OS packages, sets up Python 3.12, installs tox==4.53.0, and runs Hub unit tests (tox -e py312) with DJANGO_ANSIBLE_BASE_BRANCH set to the current commit SHA.

Changes

Cohort / File(s) Summary
GitHub Actions Workflow
\.github/workflows/dab-consumers.yml
Adds hub job: checks out ansible/galaxy_ng into hub/, installs OS libs, sets up Python 3.12, installs tox==4.53.0, sets DJANGO_ANSIBLE_BASE_BRANCH=${{ github.sha }}, and runs tox -e py312 to execute Hub unit tests.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding Hub (galaxy_ng) as a DAB consumer with GitHub Actions integration for unit testing.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jerabekjiri jerabekjiri force-pushed the add-hub-to-dab-consumers branch from ba6f429 to f634f0f Compare April 27, 2026 17:21
@jerabekjiri jerabekjiri changed the title [WIP] Add hub to dab consumers [WIP] Add Hub to dab consumers Apr 27, 2026
@jerabekjiri jerabekjiri marked this pull request as ready for review April 27, 2026 17:42
@jerabekjiri jerabekjiri changed the title [WIP] Add Hub to dab consumers Add Hub to DAB consumers Apr 27, 2026
@jerabekjiri jerabekjiri force-pushed the add-hub-to-dab-consumers branch 2 times, most recently from a057f59 to 5111749 Compare April 27, 2026 18:27
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/dab-consumers.yml:
- Around line 118-121: Remove the trailing space from the checkout path value
used with uses: actions/checkout@v4 — locate the step that sets path:
django-ansible-base (currently with a trailing space) and change it to path:
django-ansible-base so the checked-out directory name has no trailing whitespace
and sibling lookups (e.g., ../django-ansible-base) work correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 8334256e-24bd-48f2-9cc7-955f6fdc49b5

📥 Commits

Reviewing files that changed from the base of the PR and between d09e66f and a057f59.

📒 Files selected for processing (1)
  • .github/workflows/dab-consumers.yml

Comment thread .github/workflows/dab-consumers.yml Outdated
Copy link
Copy Markdown
Member

@john-westcott-iv john-westcott-iv left a comment

Choose a reason for hiding this comment

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

📋 Code Review Summary

This PR adds a Hub (galaxy_ng) job to the dab-consumers workflow. However, the job has a critical gap: it never installs the local DAB checkout into the Hub test environment, meaning it tests against whatever DAB version galaxy_ng pins internally — not the current DAB commit.

Files Reviewed: 1 file
Comments Posted: 3 review comments

🔍 Issues Found

  • 0 security/safety issues
  • 1 correctness/logic issue (DAB not installed — defeats purpose of workflow)
  • 2 code quality suggestions (missing EOF newline, SHA pinning)

Overall Assessment

Needs fix — The missing DAB installation is a blocking issue. Without it, this job does not accomplish what the dab-consumers workflow is designed to do (test current DAB against downstream consumers). Both AWX and EDA-server jobs explicitly install the local DAB; Hub needs to do the same.

General Feedback

  • The job structure follows the existing pattern well (checkout DAB, checkout consumer, setup Python, run tests)
  • Just needs the DAB installation step before running tox
  • Consider SHA-pinning actions per the direction set in PR #987

Comment thread .github/workflows/dab-consumers.yml
Comment thread .github/workflows/dab-consumers.yml
Comment thread .github/workflows/dab-consumers.yml
Copy link
Copy Markdown
Member

@john-westcott-iv john-westcott-iv left a comment

Choose a reason for hiding this comment

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

Approving with comments — please address the inline feedback (especially the DAB installation gap and missing EOF newline) before merging.

@jerabekjiri jerabekjiri force-pushed the add-hub-to-dab-consumers branch from 0d86b33 to 57a7062 Compare April 28, 2026 17:48
@jerabekjiri
Copy link
Copy Markdown
Contributor Author

confirming in ae94548 that Hub installs custom local DAB

>>> DAB CI TEST - USING LATEST DAB FROM PR <<<
============================= test session starts ==============================
...

and that the galaxy_ng is using latest DAB commit

Run tox -e py312
  tox -e py312
  shell: /usr/bin/bash -e {0}
  env:
    pythonLocation: /opt/hostedtoolcache/Python/3.12.13/x64
    ...
    DJANGO_ANSIBLE_BASE_BRANCH: eaf855f058e990917084ddcd0df35c1c29df8e59

@jerabekjiri jerabekjiri force-pushed the add-hub-to-dab-consumers branch from 57a7062 to c56e0e2 Compare April 28, 2026 18:17
@jerabekjiri
Copy link
Copy Markdown
Contributor Author

Sonarcloud still complains with

run: pip install --only-binary ':all:'  tox==4.53.0

Using dependencies without locking resolved versions is security-sensitive.

I'm inclined to mark this as false positive or safe

Copy link
Copy Markdown
Member

@john-westcott-iv john-westcott-iv left a comment

Choose a reason for hiding this comment

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

Re-Review Summary

Re-reviewing after updates. All previous comments have been addressed.

Files Reviewed: 1 file
Comments Posted: 0 new inline comments

Previous Comments Status

  1. DAB installation gap (critical) — Resolved. The approach now uses DJANGO_ANSIBLE_BASE_BRANCH: ${{ github.sha }} to pass the current DAB commit to galaxy_ng's setup.py, which installs DAB from that SHA. Author confirmed it works via CI run (ae94548). Companion galaxy_ng PR #474 is merged.

  2. Missing EOF newline — Resolved. Trailing newline is now present.

  3. SHA pinning for actions — Acknowledged. Author clarified the team decision is to only pin SHAs for unofficial actions. actions/checkout and actions/setup-python are official GitHub actions.

SonarCloud Issue

The SonarCloud "C Security Rating" failure is about pip install --only-binary ':all:' tox==4.53.0 flagged as "Using dependencies without locking." This is a false positive — the version IS pinned to an exact version, and --only-binary prevents source builds. This is a standard CI pattern. Agree with the author that this should be marked safe in SonarCloud.

Overall Assessment

LGTM — Previous blocking concerns addressed. Approval stands.

@sonarqubecloud
Copy link
Copy Markdown

@jerabekjiri jerabekjiri changed the title Add Hub to DAB consumers [AAP-73775] Add Hub to DAB consumers Apr 30, 2026
@github-actions
Copy link
Copy Markdown

DVCS PR Check Results:

Could not find JIRA key(s) in PR title, branch name, or commit messages

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 this pull request may close these issues.

2 participants