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

Change web get resources api response #10598

Merged
merged 2 commits into from
Feb 28, 2022
Merged

Conversation

kimlisa
Copy link
Contributor

@kimlisa kimlisa commented Feb 24, 2022

part of RFD 55

Description

  • Cleaned up naming to follow convention that was set later
  • Created a general listResourcesGetResponse as a consistent return value
  • Wrapped JSON response in ^ object for resources that were not wrapped to accommodate future changes. Some resources like get database, desktops, and kubes returned just as a list (which requires change in the frontend)

@kimlisa kimlisa changed the title Change get resources api response Change web get resources api response Feb 24, 2022
@kimlisa kimlisa removed the request for review from alex-kovoy February 24, 2022 21:30
@kimlisa kimlisa force-pushed the lisa/change-api-response branch from b4e381d to ca46af9 Compare February 24, 2022 22:14
Copy link
Contributor

@espadolini espadolini left a comment

Choose a reason for hiding this comment

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

e was mistakenly changed to an earlier commit, it should be left alone

@kimlisa kimlisa force-pushed the lisa/change-api-response branch from 0137417 to ca46af9 Compare February 25, 2022 16:22
@kimlisa kimlisa force-pushed the lisa/change-api-response branch from ca46af9 to 051da5e Compare February 25, 2022 16:26
@kimlisa kimlisa requested a review from espadolini February 25, 2022 16:26
Copy link
Contributor

@espadolini espadolini left a comment

Choose a reason for hiding this comment

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

Adding fields to our responses to the web UI won't break things, right?

@kimlisa
Copy link
Contributor Author

kimlisa commented Feb 25, 2022

Adding fields to our responses to the web UI won't break things, right?

@espadolini handling backwards compatibility for webassets has been very fuzzy to me. Backwards compat becomes a problem where there are mixed proxy versions: one older version can serve the old webassets, and an api call can go to a newer proxy returning a new response (and vice versa). I'm not sure how realistic it is for proxy versions to be mixed for long though, so with this assumption I did not supply fallbacks

@kimlisa kimlisa enabled auto-merge (squash) February 28, 2022 22:10
@kimlisa kimlisa merged commit 19627ee into master Feb 28, 2022
@kimlisa kimlisa deleted the lisa/change-api-response branch February 28, 2022 22:23
kimlisa added a commit that referenced this pull request Feb 28, 2022
* Created a general listResourcesGetResponse as a consistent return value
* Wrapped response in ^ object for resources that were not wrapped to 
   accommodate future changes (dbs, desktops, kubes)
kimlisa added a commit that referenced this pull request Mar 1, 2022
* Created a general listResourcesGetResponse as a consistent return value
* Wrapped response in ^ object for resources that were not wrapped to 
   accommodate future changes (dbs, desktops, kubes)
@webvictim webvictim mentioned this pull request Apr 19, 2022
@webvictim webvictim mentioned this pull request Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants