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: replace name with namespace #2787

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

agatha197
Copy link
Contributor

@agatha197 agatha197 commented Oct 29, 2024

Support for Extended Image Info

This PR introduces support for the extended image info feature, which affects how image namespaces are handled and displayed throughout the application.

Changes:

  • Added a supportExtendedImageInfo flag based on the backend client's capabilities
  • Updated image queries to use namespace instead of name when extended image info is supported
  • Modified image full name generation to prioritize namespace over name
  • Adjusted environment selection logic to use namespace when available
  • Updated table columns in MyEnvironmentPage to display and sort by namespace when supported

Rationale:

These changes allow the application to work with both the old and new image information structures, providing a smooth transition as the backend evolves.

Effects:

  • Users will see more accurate namespace information when using a backend that supports extended image info
  • Developers should be aware of the supportExtendedImageInfo flag when working with image-related components

How to test:

  1. Checkout core branch to topic/10-22-feature_add_info_field_to_gql_image_schema
  2. Check the environments, my environments, session page which are using namespace (before feat: Add info fields to GQL image schema backend.ai#2939, it was name.) of images query.

Screenshots:
In #2785, the part of the full image path field had undefined. But it has some value now.
image.png

Checklist:

  • Mention to the original issue
  • Documentation
  • Minium required manager version: 24.09.1
  • Specific setting for review
  • Minimum requirements to check during review:
    • Verify that image namespaces are correctly displayed and used when extended image info is supported
    • Ensure backwards compatibility with older backend versions
  • Test case(s) to demonstrate the difference of before/after

Copy link

graphite-app bot commented Oct 29, 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.

Copy link
Contributor Author

agatha197 commented Oct 29, 2024

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

Join @agatha197 and the rest of your teammates on Graphite Graphite

@github-actions github-actions bot added the size:M 30~100 LoC label Oct 29, 2024
@agatha197 agatha197 force-pushed the feature/new-image-parsing branch from b5b801e to e29820c Compare October 29, 2024 10:52
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch 2 times, most recently from 22364bb to 01fc2fd Compare October 30, 2024 02:03
@agatha197 agatha197 force-pushed the feature/new-image-parsing branch from e29820c to 7cc7dfa Compare October 31, 2024 03:36
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch from 01fc2fd to 047ba54 Compare October 31, 2024 03:37
@agatha197 agatha197 force-pushed the feature/new-image-parsing branch from 7cc7dfa to 8aa4dfc Compare October 31, 2024 03:42
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch from 047ba54 to 6695815 Compare October 31, 2024 03:42
@agatha197 agatha197 added this to the 24.09 milestone Oct 31, 2024
@agatha197 agatha197 requested a review from lizable October 31, 2024 06:26
@agatha197 agatha197 force-pushed the feature/new-image-parsing branch from 8aa4dfc to b75514d Compare October 31, 2024 08:02
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch from 6695815 to cbb0e50 Compare October 31, 2024 08:02
Copy link
Contributor

@ironAiken2 ironAiken2 left a comment

Choose a reason for hiding this comment

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

It works fine, but I left some small comments :)

@agatha197 agatha197 force-pushed the feature/new-image-parsing branch from b75514d to 6c80e2e Compare November 1, 2024 03:15
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch from cbb0e50 to e1139c6 Compare November 1, 2024 03:15
@github-actions github-actions bot added the size:L 100~500 LoC label Nov 1, 2024
Copy link
Member

yomybaby commented Nov 1, 2024

I got an error when manager version is 24.09.1.rc.

No data returned for operation `ImageEnvironmentSelectFormItemsQuery`, got error(s):
Cannot query field 'namespace' on type 'Image'.

See the error `source` property for more information.
RelayNetwork: No data returned for operation `ImageEnvironmentSelectFormItemsQuery`, got error(s):
Cannot query field 'namespace' on type 'Image'.

You can reproduce in dogbowl, v24.09.1.rc. Please check.

This is a trailing dot of GraphQL version directive. I fixed it and pushed.

@agatha197 agatha197 force-pushed the feature/new-image-parsing branch from 8b7d2a6 to 483a427 Compare November 1, 2024 09:45
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch 2 times, most recently from fbed260 to 92cdd3e Compare November 4, 2024 02:40
@agatha197 agatha197 mentioned this pull request Nov 4, 2024
6 tasks
@agatha197 agatha197 force-pushed the feature/new-image-parsing branch from 483a427 to b95b56b Compare November 4, 2024 06:51
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch from 92cdd3e to 143e45d Compare November 4, 2024 06:51
@agatha197 agatha197 force-pushed the feature/new-image-parsing branch from b95b56b to 07ac58f Compare November 4, 2024 07:02
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch from 143e45d to 2ba59aa Compare November 4, 2024 07:02
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

@agatha197 agatha197 force-pushed the feature/new-image-parsing branch from 07ac58f to 1aaa31b Compare November 4, 2024 07:37
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch from 2ba59aa to 7dbc971 Compare November 4, 2024 07:37
@agatha197 agatha197 modified the milestones: 24.09, 24.12 Nov 4, 2024
@agatha197 agatha197 changed the base branch from feature/new-image-parsing to graphite-base/2787 November 5, 2024 09:53
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch from 7dbc971 to 408ee75 Compare November 5, 2024 09:54
@github-actions github-actions bot added the area:lib Library and SDK related issue. label Nov 5, 2024
@agatha197 agatha197 changed the base branch from graphite-base/2787 to main November 5, 2024 09:55
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch from 408ee75 to d7d64c4 Compare November 5, 2024 09:55
Copy link

github-actions bot commented Nov 5, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements 4.92% 349/7089
🔴 Branches 4.54% 219/4824
🔴 Functions 2.86% 67/2346
🔴 Lines 4.82% 334/6935

Test suite run success

93 tests passing in 12 suites.

Report generated by 🧪jest coverage report action from 299ae73

Copy link

graphite-app bot commented Nov 5, 2024

Merge activity

# Support for Extended Image Info

This PR introduces support for the extended image info feature, which affects how image namespaces are handled and displayed throughout the application.

**Changes:**

- Added a `supportExtendedImageInfo` flag based on the backend client's capabilities
- Updated image queries to use `namespace` instead of `name` when extended image info is supported
- Modified image full name generation to prioritize `namespace` over `name`
- Adjusted environment selection logic to use `namespace` when available
- Updated table columns in MyEnvironmentPage to display and sort by `namespace` when supported

**Rationale:**

These changes allow the application to work with both the old and new image information structures, providing a smooth transition as the backend evolves.

**Effects:**

- Users will see more accurate namespace information when using a backend that supports extended image info
- Developers should be aware of the `supportExtendedImageInfo` flag when working with image-related components

**How to test:**
1. Checkout core branch to `topic/10-22-feature_add_info_field_to_gql_image_schema`
2. Check the environments, my environments, session page which are using `namespace` (before lablup/backend.ai#2939, it was `name`.) of `images` query.

**Screenshots:**
In #2785, the part of the full image path field had `undefined`. But it has some value now.
![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/2HueYSdFvL8pOB5mgrUQ/a2553afe-d555-40b3-bc4e-bb1b79fa4b87.png)

**Checklist:**

- [ ] Mention to the original issue
- [ ] Documentation
- [x] Minium required manager version: 24.09.1
- [x] Specific setting for review
- [x] Minimum requirements to check during review:
  - Verify that image namespaces are correctly displayed and used when extended image info is supported
  - Ensure backwards compatibility with older backend versions
- [ ] Test case(s) to demonstrate the difference of before/after
@agatha197 agatha197 force-pushed the feature/replace-name-with-namespace branch from d7d64c4 to 299ae73 Compare November 5, 2024 10:04
@graphite-app graphite-app bot merged commit 299ae73 into main Nov 5, 2024
6 checks passed
@graphite-app graphite-app bot deleted the feature/replace-name-with-namespace branch November 5, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:lib Library and SDK related issue. size:L 100~500 LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants