Skip to content

Remove image and registry info from PO analysis start request - #232

Merged
brucetony merged 4 commits into
mainfrom
230-stop-compiling-registry-credentials-for-the-po
Oct 30, 2026
Merged

brucetony merged 4 commits into
mainfrom
230-stop-compiling-registry-credentials-for-the-po

Conversation

@brucetony

@brucetony brucetony commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Requests that retrieve analysis nodes can now include related analysis data. Query parameters also support comma-separated include values.
  • Changes
    • Analysis creation requests now use the analysis ID, project ID, and Kong token. Registry URLs, registry credentials, and image URLs are no longer included.
    • Analysis startup no longer retrieves node or registry metadata before preparing a request.

@brucetony brucetony linked an issue Oct 28, 2026 that may be closed by this pull request
@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Walkthrough

The adapter removes registry and image metadata lookups and fields from the PO analysis start request. The request retains the analysis ID, project ID, and Kong token.

Changes: PO analysis payload

Layer / File(s) Summary
Request contract and payload compilation
hub_adapter/schemas/podorc.py, hub_adapter/routers/podorc.py, hub_adapter/dependencies.py, hub_adapter/constants.py, hub_adapter/schemas/hub.py
The request schema and route no longer include registry credentials or image URL fields. The payload compiler returns the analysis ID, project ID, and Kong token. Related registry constants and Hub schema models were removed.
Autostart integration and validation
hub_adapter/autostart.py, tests/test_autostart.py, tests/test_dependencies.py, tests/constants.py
Autostart calls the compiler without node or registry metadata lookups. Tests now cover the reduced call and payload; the mock node fixture no longer passes robot_id.

Priority: Not assessed

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Refactor

Possibly related issues

Possibly related PRs

Merge Risk: Minimal · up to 26f80

Node and project validation gaps warrant correction, but available evidence does not establish that this change introduces or worsens them. No new merge-blocking failure was demonstrated.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
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.
Title check Passed The title clearly and concisely describes the main change: removing image and registry information from the PO analysis start request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 5 files.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@tada5hi

tada5hi Bot commented Oct 28, 2026

Copy link
Copy Markdown

Merge PrivateAIM/node-hub-api-adapter#‍232?

This is a human-authored PR, not a Dependabot or Renovate PR, so automatic merging is not allowed. It removes image and registry information from the PO analysis start request. No dependency updates were identified. All checks are successful (none failing, pending, or missing). Approve merging this head?

Head: f01405c0654548a4c32541e9bf25ff183b0ec0cc

Waiting for approval until 2026-10-29T13:19:23.533Z. Approve or deny: https://maintainer.placzek.dev/ui/approvals?run=bfdf60dd-5bec-48af-ade8-6e895cecb257&approval=1%3Acall_3fc5cf81c744448d837cf34b19098a8c%3A0
Run bfdf60dd-5bec-48af-ade8-6e895cecb257 of agent maintainer.

@tada5hi

tada5hi commented Oct 28, 2026

Copy link
Copy Markdown

perfect!
Thanks a lot

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔇 Additional comments: 1
hub_adapter/routers/podorc.py-48-50: 1

48-50: Minor · Potential issue · Heavy lift

⚠️ Unverified finding
We couldn't verify this from the code. It is shown for review, not as a verified issue.

Removing registry credentials makes a successful private-image start depend on the PO obtaining credentials by another means, even if it accepts the reduced request.

The pod cannot pull its image and the analysis does not start.

This can occur when: An analysis uses a private registry whose pull credentials were previously supplied in the start request.


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e92affd3-65db-4173-aa6b-760c3f96e903
Commits

Reviewing files that changed from the base of the PR and between f01405c and 518c440.

Files selected for processing: 6
  • hub_adapter/autostart.py
  • hub_adapter/constants.py
  • hub_adapter/routers/podorc.py
  • tests/constants.py
  • tests/test_autostart.py
  • tests/test_dependencies.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@tada5hi tada5hi changed the title Remove image and registry info from PO analysis statr request Remove image and registry info from PO analysis start request Oct 29, 2026
@tada5hi

tada5hi Bot commented Oct 29, 2026

Copy link
Copy Markdown

Merge PrivateAIM/node-hub-api-adapter#‍232?

This human-authored PR removes image and registry information from the PO analysis start request. Automatic merging is not allowed because it is not a Dependabot or Renovate PR. No dependency updates were detected (bump kind: unknown). All reported checks are successful, with no failing, pending, or missing checks. Approve merging this exact head?

Head: 518c4407fca0a4f403212bd32c7cf106840a578c

Waiting for approval until 2026-10-30T13:19:53.287Z. Approve or deny: https://maintainer.placzek.dev/ui/approvals?run=24709c41-7edb-4854-9ddf-2c0f16382e10&approval=1%3Acall_64b9240b1ff84d5d8e9d175894292774%3A0
Run 24709c41-7edb-4854-9ddf-2c0f16382e10 of agent maintainer.

@brucetony
brucetony merged commit 50fe37b into main Oct 30, 2026
2 checks passed
@brucetony
brucetony deleted the 230-stop-compiling-registry-credentials-for-the-po branch October 30, 2026 11:37
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.

Stop compiling registry credentials for the PO

2 participants