-
Notifications
You must be signed in to change notification settings - Fork 96
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
OCM-7800 | Define and add pagination model to cloud_provider_inquiries #950
base: main
Are you sure you want to change the base?
OCM-7800 | Define and add pagination model to cloud_provider_inquiries #950
Conversation
89f6dbb
to
c2d28fc
Compare
*/ | ||
|
||
// Pagination contains the data which enables pagination on the response | ||
struct Pagination { |
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.
Not that big a fan of this as it is specific only for one model.
How do other models have the pagination implemented?
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.
BreakGlassCredentialsListRequest has implemented pagination the request struct like this: https://github.com/openshift-online/ocm-sdk-go/blob/d6662dfa3e339336e2459a413289c5b867500131/clustersmgmt/v1/break_glass_credentials_client.go#L350
I think this way it's more pluggable to any struct that needs to implement pagination on the response.
lgtm |
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.
This seems wrong. We use a *_resource.model
that automatically adds pagination components using method List
. You might want to follow what's being done in https://github.com/openshift-online/ocm-api-model/blob/main/model/clusters_mgmt/v1/break_glass_credentials_resource.model#L20-L62 to support this.
c2d28fc
to
f392204
Compare
Signed-off-by: Chetan Giradkar <[email protected]>
f392204
to
ad9b5cc
Compare
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.
This builds well now and generates the following files in the SDK:
clustersmgmt/v1/cloud_provider_data_client.go
clustersmgmt/v1/cloud_provider_data_resource_json.go
clustersmgmt/v1/cloud_providers_data_client.go
clustersmgmt/v1/cloud_providers_data_resource_json.go
Is there a specific endpoint in CS associated with this or does it expect the /{aws,gcp}_inquiries/*
endpoints?
The original task had broader consideration, but this PR only aims to add pagination structure to |
No description provided.