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

hotfix: regression of image info display in serving page #2244

Merged

Conversation

lizable
Copy link
Contributor

@lizable lizable commented Mar 6, 2024

Checklist: (if applicable)

  • Mention to the original issue
  • Documentation
  • Minimum 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

This PR fixes two thing,

  • Show full-image string in Routing page, as a regression of replacing deprecated image key in endpoint to image_object.

  • Supports architecture on same image string (multiarch) by extracting from full-image string

  • Core(manager) version: 23.9.9~

How to test:

  1. Go to Serving page
  2. Click Start service button.
  3. Service launcher modal pops up.
  4. Create a service with two cases:
    • Select from environment/version
    • Input image string (cr.backend.ai/multiarch/python:3.9-ubuntu20.04@aarch64) in manual image input field
  5. Click Create button.
  6. Click Endpoint Name you just created from step 4.
  7. See image info in Routing page.
After Before
Screenshot 2024-03-06 at 12 02 14 PM Screenshot 2024-03-06 at 12 24 07 PM

@lizable lizable added area:ux UI / UX issue. urgency:blocker IT SHOULD BE RESOLVED BEFORE DISTRIBUTING labels Mar 6, 2024
@lizable lizable added this to the 23.09 milestone Mar 6, 2024
@lizable lizable self-assigned this Mar 6, 2024
@lizable lizable requested a review from ironAiken2 March 6, 2024 03:23
@github-actions github-actions bot added the size:M 30~100 LoC label Mar 6, 2024
Copy link

github-actions bot commented Mar 6, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements
2.9% (-0% 🔻)
102/3512
🔴 Branches
3.09% (-0.01% 🔻)
69/2235
🔴 Functions 1.44% 17/1179
🔴 Lines
2.96% (-0% 🔻)
102/3448

Test suite run success

20 tests passing in 4 suites.

Report generated by 🧪jest coverage report action from 9b35fe4

@@ -113,7 +113,26 @@ const RoutingListPage: React.FC<RoutingListPageProps> = () => {
name
status
endpoint_id
image
Copy link
Member

Choose a reason for hiding this comment

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

To ensure backward compatibility, we should use different images based on the version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just added conditional statement to check whether current core version supports image_object key in endpoint.

Copy link
Member

Choose a reason for hiding this comment

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

Please add @deprecatedSince like this:
image @deprecatedSince(version: "23.09.9")

@lizable lizable requested a review from yomybaby March 6, 2024 05:31
@@ -113,7 +113,26 @@ const RoutingListPage: React.FC<RoutingListPageProps> = () => {
name
status
endpoint_id
image
Copy link
Member

Choose a reason for hiding this comment

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

Please add @deprecatedSince like this:
image @deprecatedSince(version: "23.09.9")

Comment on lines 275 to 276
image
image_object @since(version: "23.09.9") {
Copy link
Member

Choose a reason for hiding this comment

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

Thia page doesn't use image and image_object, right?. If yes, please remove these in this file.

@lizable lizable requested a review from yomybaby March 6, 2024 07:38
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

@yomybaby yomybaby merged commit 59c9163 into main Mar 6, 2024
8 of 9 checks passed
@yomybaby yomybaby deleted the hotfix/regression-of-image-info-display-in-serving-page branch March 6, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:ux UI / UX issue. size:M 30~100 LoC urgency:blocker IT SHOULD BE RESOLVED BEFORE DISTRIBUTING
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants