-
Notifications
You must be signed in to change notification settings - Fork 151
Fix catalog source preview: pass yamlCatalogPath as a fallback in case yaml content is missing (default sources), pass type-specific properties under properties sub-object
#2033
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
properties sub-object
ppadti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this with cluster, works fine.
just a minor comment
| } | ||
| } | ||
|
|
||
| func CreateSampleCatalogSource(id string, name string, catalogType string, enabled bool) models.CatalogSourceConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we are not using this CreateSampleCatalogSource we can remove this...
ederign
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
… request Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
602a5fb to
dd21e6e
Compare
|
Thank you Mike! |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ederign The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
🤖 Description written in part by Claude Code
This PR adds support for tracking and using the
yamlCatalogPath(the ConfigMap key where YAML catalog data is stored) in addition to the YAML content itself. This enables us to preview default catalog sources which don't have their YAML content embedded in the configmap by passing theyamlCatalogPaththe API can use to look up the existing yaml.Edit: Related bug fix: the preview request also wasn't passing source-type-specific properties (allowedOrganization, apiKey, yamlCatalogPath) under a "properties" sub-object as required.
Backend Changes (BFF)
Model Updates
YamlCatalogPath *stringfield toCatalogSourceConfigtypeAPI Behavior
yamlCatalogPathalong with YAML content when fetching individual source configsyamlcontent as a multipart form file (for new sources or when YAML is modified), ORyamlCatalogPathin the config JSON (for existing sources where YAML hasn't changed)allowedOrganization,apiKey, andyamlCatalogPathunder a.propertiessub-object when constructing the config JSON sent to the catalog serviceOther Improvements
Frontend Changes
Type System
yamlCatalogPath?: stringtoYamlCatalogSourceConfiginterfaceComponent Updates
existingSourceConfigparameter and preservesyamlCatalogPathfrom existing configsyamlandyamlCatalogPathpropertiescatalogSourceConfigToFormDatacall intoManageSourceForm, eliminating redundantexistingDatapropTesting
yamlCatalogPathonlyyamlfalling back toyamlCatalogPathyamlCatalogPaththrough form transformationsMigration Notes
This is a backward-compatible change:
yamlcontent will continue to workyamlCatalogPathas a fallbackHow Has This Been Tested?
Tested mock mode continues to work, and locally synced changes to ODH to test against production downstream catalog service
Merge criteria:
DCOcheck)ok-to-testhas been added to the PR.If you have UI changes