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: new image parsing on Environment page #2785

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

agatha197
Copy link
Contributor

@agatha197 agatha197 commented Oct 29, 2024

related PR: lablup/backend.ai#2939

Extended Image Information Support

This PR adds support for extended image information in the ImageList component and related translations. It introduces new fields and modifies the display of image details when the backend supports the 'extended-image-info' feature.

Checklist:

  • Minimum required manager version: 24.09.1

Changes

  1. Updated ImageListQuery to include new fields: namespace, base_image_name, tags, and version.
  2. Modified the ImageList component to conditionally render columns based on the supportExtendedImageInfo flag.
  3. Added new translations for "Base image name" and "Tags" in multiple languages.
  4. Updated the backend client to support the 'extended-image-info' feature for manager version 24.09.1 and above.

Impact

  • Users will see more detailed image information when using a compatible backend version.
  • The image list will display new columns for base image name and tags when supported.
  • Existing functionality is preserved for older backend versions.

Review Notes

  • Verify that the image list displays correctly with both old and new backend versions.
  • Check that new translations are applied correctly in different languages.
  • Ensure that sorting and filtering work properly with the new fields.

How to test

  1. Checkout core branch to topic/10-22-feature_add_info_field_to_gql_image_schema fix: Strengthen join condition between kernels and images backend.ai#2993
  2. Go to Environment page.

What to check:

  • Check the data is valid.
    • Since 24.09.1: Full image path, Registry, Architecture, Namespace, Base image name, Version, Tags, Digest, Resource limit, Control.
    • Before 24.09.1: Full image path, Registry, Architecture, Name, Language, Version, Base, Constraints, Digest, Resource limit, Control.
  • All data is highlightable by searching images.
  • Sortable data works fine.

Screenshots

image.png

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 area:lib Library and SDK related issue. area:ux UI / UX issue. area:i18n Localization size:L 100~500 LoC labels Oct 29, 2024
Copy link

github-actions bot commented Oct 29, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements 4.92% 349/7099
🔴 Branches 4.5% 218/4844
🔴 Functions 2.86% 67/2345
🔴 Lines 4.81% 334/6942

Test suite run success

93 tests passing in 12 suites.

Report generated by 🧪jest coverage report action from b0d7ea2

@agatha197 agatha197 added this to the 24.09 milestone Oct 29, 2024
@agatha197 agatha197 changed the title feat: new image parsing feat: new image parsing on Environment page Oct 29, 2024
@agatha197 agatha197 force-pushed the feature/new-image-parsing branch 3 times, most recently from cffcb89 to 3e63d3e Compare October 29, 2024 07:28
@agatha197 agatha197 marked this pull request as draft October 29, 2024 07:47
@github-actions github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Oct 29, 2024
@agatha197 agatha197 force-pushed the feature/new-image-parsing branch 2 times, most recently from 584f0dc to b5b801e Compare October 29, 2024 09:09
@agatha197 agatha197 mentioned this pull request Oct 29, 2024
6 tasks
@agatha197 agatha197 marked this pull request as ready for review October 29, 2024 09:17
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.

(dismissed)

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.

Please change the Table Column name to start with an uppercase letter... sorry I missed it.

e.g.) Full image path -> Full Image Path

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 check my suggestion.

react/src/hooks/index.tsx Outdated Show resolved Hide resolved
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 mentioned this pull request Nov 4, 2024
6 tasks
@agatha197 agatha197 force-pushed the feature/new-image-parsing branch 3 times, most recently from 07ac58f to 1aaa31b Compare November 4, 2024 07:37
Copy link
Contributor Author

Please change the Table Column name to start with an uppercase letter... sorry I missed it.

e.g.) Full image path -> Full Image Path

I followed Backend.AI Writing Rules 😉

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.

LGTM

Copy link

graphite-app bot commented Nov 5, 2024

Merge activity

related PR: lablup/backend.ai#2939

# Extended Image Information Support

This PR adds support for extended image information in the ImageList component and related translations. It introduces new fields and modifies the display of image details when the backend supports the 'extended-image-info' feature.

**Checklist:**
- [ ] Minimum required manager version: 24.09.1

## Changes

1. Updated `ImageListQuery` to include new fields: `namespace`, `base_image_name`, `tags`, and `version`.
2. Modified the ImageList component to conditionally render columns based on the `supportExtendedImageInfo` flag.
3. Added new translations for "Base image name" and "Tags" in multiple languages.
4. Updated the backend client to support the 'extended-image-info' feature for manager version 24.09.1 and above.

## Impact

- Users will see more detailed image information when using a compatible backend version.
- The image list will display new columns for base image name and tags when supported.
- Existing functionality is preserved for older backend versions.

## Review Notes

- Verify that the image list displays correctly with both old and new backend versions.
- Check that new translations are applied correctly in different languages.
- Ensure that sorting and filtering work properly with the new fields.

## How to test
1. Checkout core branch to `topic/10-22-feature_add_info_field_to_gql_image_schema` lablup/backend.ai#2993
2. Go to Environment page.

## What to check:
- [ ] Check the data is valid.
  - [ ]  Since 24.09.1: Full image path, Registry, Architecture, Namespace, Base image name, Version, Tags, Digest, Resource limit, Control.
  - [ ] Before 24.09.1: Full image path, Registry, Architecture, Name, Language, Version, Base, Constraints, Digest, Resource limit, Control.
- [ ] All data is highlightable by searching images.
- [ ] Sortable data works fine.

## Screenshots

![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/2HueYSdFvL8pOB5mgrUQ/88e5ad80-559a-4b41-a93a-5a08f3a4c062.png)
@graphite-app graphite-app bot merged commit b0d7ea2 into main Nov 5, 2024
6 checks passed
@graphite-app graphite-app bot deleted the feature/new-image-parsing branch November 5, 2024 09:54
agatha197 added a commit that referenced this pull request Nov 5, 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 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
yomybaby pushed a commit that referenced this pull request Nov 12, 2024
**Changes:**
Refactored image handling similar to #2785 and #2795 for versions before 24.12. Before 24.12, most data parsing is handled on the frontend.

Key changes:
- Consolidated separate tag components (BaseImageTags, ConstraintTags, LangTags) into a single ImageTags component
- Simplified image metadata extraction by using string operations instead of complex parsing
- Removed redundant language column and consolidated information into Tags column
- Added new getTags utility function to consistently parse and format image tag information
- Updated image search functionality to search through tag keys and values

**Impact:**
- Cleaner, more consistent display of image metadata in the UI
- More maintainable code structure for handling image tags
- Improved search capabilities across image metadata

**Screenshots:**

![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/2HueYSdFvL8pOB5mgrUQ/483e263b-88bc-4d5e-8a22-d3e219695f76.png)

**What to check:**
Data parsing is the same as 24.12.

**Checklist:**
- [ ] Mention to the original issue
- [ ] Documentation
- [x] Minium required manager version: manager < 24.12
- [x] Specific setting for review: 10.100.64.15
- [x] Minimum requirements to check during review
- [ ] Test case(s) to demonstrate the difference of before/after: will be handled in another stack
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:i18n Localization area:lib Library and SDK related issue. area:ux UI / UX issue. size:XL 500~ LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants