Skip to content
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

feat: support for extra field in registry type #2927

Merged
merged 1 commit into from
Dec 23, 2024

Conversation

ironAiken2
Copy link
Contributor

@ironAiken2 ironAiken2 commented Dec 9, 2024

This PR resolves #2914 issue

related Core PR: #3208

24.09~24.09.2 : Using container_registry_nodes but does not support extra fields.
24.09.3 ~ : support extra field

Changes:
Extra field and argument are supported from version 24.09.3 onwards.
Because the arguments for each mutation are not provided as input types, you must create a separate mutation if you want to exclude a specific argument.

  • Added extra field to container registry nodes to support additional JSON metadata
  • Added JSON editor UI component for managing extra information in container registry editor
  • Added validation for proper JSON formatting with error messages
  • Made value and onChange props optional in BAICodeEditor component
  • Added translations for JSON format validation messages across all languages

How to test:

  1. move to registry page
  2. enter Extra Information to verify that the request succeeds only for valid JSON types.
dark light
image.png image.png

Checklist: (if applicable)

  • Mention to the original issue
  • Documentation
  • Minium required manager version
  • Specific setting for review (eg., KB link, endpoint or how to setup)
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after

Copy link

graphite-app bot commented Dec 9, 2024

Your org requires the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added area:ux UI / UX issue. area:i18n Localization size:L 100~500 LoC labels Dec 9, 2024
Copy link
Contributor Author

ironAiken2 commented Dec 9, 2024


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ironAiken2 ironAiken2 force-pushed the feat/support-extra-field-in-registry branch from 5009cbd to 6baf617 Compare December 9, 2024 05:27
Copy link

github-actions bot commented Dec 9, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements
5.16% (-0.01% 🔻)
398/7707
🔴 Branches
4.45% (-0.02% 🔻)
237/5320
🔴 Functions
3.09% (-0% 🔻)
78/2525
🔴 Lines
5.08% (-0.01% 🔻)
383/7539

Test suite run success

124 tests passing in 14 suites.

Report generated by 🧪jest coverage report action from d288132

@ironAiken2 ironAiken2 force-pushed the feat/support-extra-field-in-registry branch from 6baf617 to 9708910 Compare December 9, 2024 05:30
@ironAiken2 ironAiken2 marked this pull request as ready for review December 9, 2024 05:40
@ironAiken2 ironAiken2 force-pushed the feat/support-extra-field-in-registry branch from 9708910 to 2725067 Compare December 9, 2024 05:41
@yomybaby yomybaby marked this pull request as draft December 9, 2024 09:04
@ironAiken2 ironAiken2 marked this pull request as ready for review December 9, 2024 09:07
@ironAiken2 ironAiken2 marked this pull request as draft December 9, 2024 09:07
Copy link
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

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

Please add a condition for version compatibility.
I got an error below on v24.09.2

No data returned for operation `ContainerRegistryListQuery`, got error(s): Cannot query field 'extra' on type 'ContainerRegistryNode'. See the error `source` property for more information.

@ironAiken2 ironAiken2 force-pushed the feat/support-extra-field-in-registry branch from 2725067 to ff501e1 Compare December 10, 2024 02:50
@github-actions github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Dec 10, 2024
@ironAiken2 ironAiken2 marked this pull request as ready for review December 10, 2024 04:14
@ironAiken2 ironAiken2 requested a review from yomybaby December 10, 2024 04:23
@yomybaby yomybaby force-pushed the feat/support-extra-field-in-registry branch from ff501e1 to a268421 Compare December 10, 2024 07:58
@ironAiken2 ironAiken2 force-pushed the feat/support-extra-field-in-registry branch 3 times, most recently from e91b22d to 656249b Compare December 10, 2024 10:01
@yomybaby yomybaby requested a review from agatha197 December 13, 2024 06:14
Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

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

I'm not sure why you used isEmpty and isNil together, but everything else looks good to me. Good!

react/src/components/ContainerRegistryEditorModal.tsx Outdated Show resolved Hide resolved
@ironAiken2 ironAiken2 force-pushed the feat/support-extra-field-in-registry branch from 656249b to 96d4d8d Compare December 16, 2024 04:26
@ironAiken2 ironAiken2 requested a review from yomybaby December 16, 2024 04:26
Copy link
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

graphite-app bot commented Dec 23, 2024

Merge activity

<!--
Please precisely, concisely, and concretely describe what this PR changes, the rationale behind codes,
and how it affects the users and other developers.
-->

### This PR resolves [#2914](#2914) issue

related Core PR: [#3208](lablup/backend.ai#3208)

24.09~24.09.2 : Using `container_registry_nodes` but does not support extra fields.
24.09.3 ~ : support extra field

**Changes:**
Extra field and argument are supported from version 24.09.3 onwards.
Because the arguments for each mutation are not provided as input types, you must create a separate mutation if you want to exclude a specific argument.

- Added `extra` field to container registry nodes to support additional JSON metadata
- Added JSON editor UI component for managing extra information in container registry editor
- Added validation for proper JSON formatting with error messages
- Made `value` and `onChange` props optional in BAICodeEditor component
- Added translations for JSON format validation messages across all languages

**How to test:**
1. move to registry page
2. enter Extra Information to verify that the request succeeds only for valid JSON types.

|dark|light|
|---|---|
|![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/lSyr8xXz1wdXALkJKzVx/2d12bdad-402f-4fdf-a3ca-f3a33c7839a2.png)|![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/lSyr8xXz1wdXALkJKzVx/1b44f9a6-384d-4f6d-9d3b-a51ed8b79e51.png)|

**Checklist:** (if applicable)

- [ ] Mention to the original issue
- [ ] Documentation
- [ ] Minium required manager version
- [ ] Specific setting for review (eg., KB link, endpoint or how to setup)
- [ ] Minimum requirements to check during review
- [ ] Test case(s) to demonstrate the difference of before/after
@yomybaby yomybaby force-pushed the feat/support-extra-field-in-registry branch from 96d4d8d to d288132 Compare December 23, 2024 05:45
@graphite-app graphite-app bot merged commit d288132 into main Dec 23, 2024
7 checks passed
@graphite-app graphite-app bot deleted the feat/support-extra-field-in-registry branch December 23, 2024 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:i18n Localization area:ux UI / UX issue. size:XL 500~ LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding extra input boxes to support the newly added registry types
3 participants