-
Notifications
You must be signed in to change notification settings - Fork 365
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
upcoming: [M3-8014] - Update APIv4 and Validation packages based on Image Service spec #10541
upcoming: [M3-8014] - Update APIv4 and Validation packages based on Image Service spec #10541
Conversation
Coverage Report: ✅ |
@@ -12,7 +12,7 @@ export const baseImageSchema = object({ | |||
label: labelSchema.notRequired(), | |||
description: string().notRequired().min(1).max(65000), | |||
cloud_init: boolean().notRequired(), | |||
tags: array(string()).notRequired(), | |||
tags: array(string().min(3).max(50)).max(500).notRequired(), |
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.
I wonder if we have a validation schema we can share across linodes, volumes, etc for tags
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.
Are the requirements for tags always the same across the API?
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.
I'm not 100% sure, but I would assume they are. Might require some investigation
| 'pending deletion' | ||
| 'pending replication' |
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.
Maybe we should ask for verification on this... It would be very unusual to have a status with spaces instead of underscores so I'm surprised this is what was reflected in the spec
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.
++
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.
Confirmed by Priyank Patel on API team
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.
Looks good based on the API spec ✅
| 'pending deletion' | ||
| 'pending replication' |
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.
++
2514286
to
ee9d693
Compare
Description 📝
Updates the
api-v4
andvalidation
packages to reflect the API spec.Changes 🔄
total_size
andregions
field toImage
typeImageRegion
'distributed-images'
updateImageRegions
SDK methodupdateImageRegionsSchema
Target release date 🗓️
6/10
How to test 🧪
Prerequisites
Note
These changes are only available in alpha. Additionally, this functionality is not working currently as image service development is in progress.
total_size
andregions
fields matches what is returned by the APIupdateImageRegions
API method results in a 200 response code