fix: auto-detect hub type when catalog type is empty#2353
Conversation
Summary of ChangesHello @zakisk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant improvement to how catalog types are determined. By implementing an intelligent auto-detection mechanism, the system can now dynamically identify whether an unspecified catalog is an Artifact Hub or a Tekton Hub, thereby enhancing the robustness and flexibility of catalog integration without requiring explicit configuration. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🔍 PR Lint Feedback
|
There was a problem hiding this comment.
Code Review
This pull request introduces a helpful feature to auto-detect the hub type when it's not explicitly configured. The implementation correctly probes the Artifact Hub and falls back to Tekton Hub, and the new logic is well-covered by tests. I have a couple of suggestions to improve the code by fixing a typo and adding logging for better debuggability.
5f52f32 to
32ac3f5
Compare
|
@zakisk We are using PAC SyncConfig function in the Operator here I think we should set the hub-catalog-type here, WDYT? cc: @vdemeester |
yeah, but IMO I am not trying to set |
yes, I see you are just deciding the hub-catalog-type but my point is, is this a good place to do it as this line might set hub-catalog-type to artifacthub in case of empty hub-catalog-type? |
|
@pratap0007 Yes, but in that case if user has not provided url PaC must proceed with ArtifactHub as default hub. we're doing this fix because user has configured custom hub and we're figuring out the HUB type. |
@zakisk sorry, I mentioned wrong code please check this one https://github.com/openshift-pipelines/pipelines-as-code/blob/main/pkg/params/settings/default.go#L21 |
32ac3f5 to
0e7b423
Compare
0e7b423 to
0ef343c
Compare
When the catalog type is not specified, the code now probes the Artifact Hub stats endpoint to determine the catalog type. If the endpoint responds successfully, the catalog is treated as Artifact Hub; otherwise, it falls back to Tekton Hub. - Call /api/v1/stats endpoint to detect Artifact Hub catalogs - Fallback to Tekton Hub if the API call fails - Update tests with HTTP mock client for catalog type detection https://issues.redhat.com/browse/SRVKP-9248 Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
0ef343c to
2c7398c
Compare
|
LGTM |
|
this is to handle upgrade from OSP right? if the user don't have hub-type set then it will always end up trying to detect on every request (and due adding one http call more everytime to hub)..... @zakisk i think we can update the configmap instead to the right hub-type so we don't do another call again and again on every payload. @pratap0007 better way would be to have the operator doing this, i think you added some logic to do some work on osp upgrade, can you try to reuse this code we have here to detect the hub type and set it properly in the configmap from the operator? |
yeah, makes sense! |
📝 Description of the Change
When the catalog type is not specified, the code now probes the Artifact Hub stats endpoint to determine the catalog type. If the endpoint responds successfully, the catalog is treated as Artifact Hub; otherwise, it falls back to Tekton Hub.
👨🏻 Linked Jira
https://issues.redhat.com/browse/SRVKP-9248
🔗 Linked GitHub Issue
Fixes #
🚀 Type of Change
fix:)feat:)feat!:,fix!:)docs:)chore:)refactor:)enhance:)deps:)🧪 Testing Strategy
🤖 AI Assistance
If you have used AI assistance, please provide the following details:
Which LLM was used?
Extent of AI Assistance:
Important
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Gemini gemini@google.com
Co-authored-by: ChatGPT noreply@chatgpt.com
Co-authored-by: Claude noreply@anthropic.com
Co-authored-by: Cursor noreply@cursor.com
Co-authored-by: Copilot Copilot@users.noreply.github.com
**💡You can use the script
./hack/add-llm-coauthor.shto automatically addthese co-author trailers to your commits.
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.