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

[DevCenter - Microsoft Dev Box] API Review #24099

Closed
azure-sdk opened this issue May 22, 2023 · 3 comments · Fixed by #24291
Closed

[DevCenter - Microsoft Dev Box] API Review #24099

azure-sdk opened this issue May 22, 2023 · 3 comments · Fixed by #24291
Assignees
Labels
API Review Scoping This is an issue that will track work on a specific set of API changes.

Comments

@azure-sdk
Copy link
Collaborator

azure-sdk commented May 22, 2023

New API Review meeting has been requested.

Service Name: DevCenter - Microsoft Dev Box
Review Created By: Chris Miller
Review Date: 06/06/2023 01:00 PM PT
Onboarding Record: https://dev.azure.com/azure-sdk/Release/_workitems/edit/10882
PR: #24291
Hero Scenarios Link: Not Provided
Core Concepts Doc Link: Not Provided

Description: Adding 2023-07-01-preview API version.

Additions:

  • Adding Environment Actions APIs
  • Adding Schedule list endpoints
  • Adding Dev Box troubleshoot API
  • Standardizing Id property on resource responses

Detailed meeting information and documents provided can be accessed here
For more information that will help prepare you for this review, the requirements, and office hours, visit the documentation here

@azure-sdk azure-sdk added the API Review Scoping This is an issue that will track work on a specific set of API changes. label May 22, 2023
@azure-sdk
Copy link
Collaborator Author

Meeting updated by Chris Miller

Service Name: DevCenter - Microsoft Dev Box
Review Created By: Chris Miller
Review Date: 06/06/2023 01:00 PM PT
Onboarding Record: https://dev.azure.com/azure-sdk/Release/_workitems/edit/10882
PR: #24291
Hero Scenarios Link: Not Provided
Core Concepts Doc Link: Not Provided

Description: Adding 2023-07-01-preview API version. PR to follow once we complete final internal review.

Additions:

  • Adding Environment Actions APIs
  • Adding Schedule list endpoints
  • Adding Dev Box troubleshoot API
  • Standardizing Id property on resource responses

Detailed meeting information and documents provided can be accessed here
For more information that will help prepare you for this review, the requirements, and office hours, visit the documentation here

@mikekistler
Copy link
Member

Notes from REST API Review meeting 6/6/23

  • How will troubleshooting actions version over time? Troubleshooting is a very general term that could cover just diagnosing to performing actions -- might be too general.
  • "properties" as the result in the LRO status monitor -- why not called "result"?
  • Consider accepting an "Operation-id" header parameter on LROs to make them idempotent
  • Clean up of status monitors should be a high-priority item.
  • id property was added as "required" -- change to optional? Leave as required unless SDK breaking.
    • Note that id is not an ARM id, to avoid potential confusion
    • Is the structure of the ID really helpful? Maybe better to have a full URI
  • Why use sourceId and not poolId ? Better to have multiple properties with specific names.
    • consider consistency with actions
    • maybe have a sourceId builder/parser if you keep it
  • Time zones ? Need these to consider location of dev boxes
    • documented as "The IANA timezone id at which the schedule should execute." -- not an enum
  • How to troubleshoot when a troubleshoot operation is outstanding?
  • lastOperationId -- user determines that a troubleshoot operation is outstanding from the state, and then queries the operation to determine the state
    • lastOperationId is actually a URL. That needs to be clarified.
    • Maybe change to troubleshoot results
    • Or make a "troubleshoot" a proper resource

@chrissmiller
Copy link
Contributor

chrissmiller commented Jun 20, 2023

Adding @markweitzel , @mikekistler , and @JeffreyRichter per our discussion in the 6/7 office hours, so we can continue the conversation on this API with the same group of people.

We did some internal alignment on our team and have made the following updates in the linked PR based on the discussion in the review and the discussion in office hours. Please let us know any thoughts and concerns; we are more than happy to set up an ad-hoc meeting or jump back into office hours to hash out any continuing problems.

Endpoint changes

  • Based on the feedback around the Environment list operations API in office hours, we will align Dev Box and Environments by standing up an equivalent list operations API on the Dev Box resource. This will follow the discussed approach for Environments of having a single polymorphic endpoint, and we'll follow along with customers during the preview to evaluate their use/any pain points this produces.
  • Updated the Environment operation get logs endpoint to return a plain text file rather than a plain text string response for easier customer consumption/reuse, given that the logs output could be quite large.
    • Discussed in office hours that we cannot use a direct blob storage URI due to network isolation requirements

Property changes

  • Updated the id property to uri everywhere, and made these full dataplane URIs. We are open to naming suggestions here; I've seen this property called resourceId, resourceLocation, and resourceUri/uri in other dataplane APIs, so not certain what the best practice is. We've left existing id properties in place to avoid a breaking change.
  • Removed lastOperationId, per your guidance that this is not a great pattern
  • Added sourceType enums to both resources with sourceUri properties (schedules and Dev Box actions) to surface the type of the source resource
    • Note: Leaving the old sourceId on actions to avoid a breaking change, and adding sourceUri to align with the URI changes.

Naming changes

  • Renamed troubleshoot to repair to reflect that we will take actions on the customer VMs
  • Updated type to kind on Environment and Dev Box operations APIs. Leveraging this as a polymorphic discriminator for both APIs.

From a behavioral perspective:

  • The work to implement an operations list API will also align well with supporting Operation-Id header because it will improve our control over tracking and fetching operations. This should enable us to add support for the idempotency pattern, and we have added a story to tackle this to our planning for next quarter so it can be included in our next API release.
  • The operations API work will also improve our ability to automatically clean up status monitors because we can leverage TTL features in cosmos, and this work has been added to our planning for next quarter as well. For our new operations APIs, we've added explicit descriptions that the operations have a 90 day TTL.

@github-project-automation github-project-automation bot moved this from Triage to Done in API Stewardship Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Review Scoping This is an issue that will track work on a specific set of API changes.
Projects
Status: Done
7 participants