Adding new ACA Session Data Plane APIs #31996

shivamkm07 · 2025-01-06T03:16:04Z

Data Plane API Specification Update Pull Request

Related PR: https://msazure.visualstudio.com/One/_git/AAPT-Antares-Containerapps/pullrequest/11352385

Tip

Overwhelmed by all this guidance? See the Getting help section at the bottom of this PR description.

PR review workflow diagram

Please understand this diagram before proceeding. It explains how to get your PR approved & merged.

API Info: The Basics

Most of the information about your service should be captured in the issue that serves as your API Spec engagement record.

Link to API Spec engagement record issue:

Is this review for (select one):

a private preview
a public preview
GA release

Change Scope

^{This section will help us focus on the specific parts of your API that are new or have been modified.
Please share a link to the design document for the new APIs, a link to the previous API Spec document (if applicable), and the root paths that have been updated.}

Design Document:
Previous API Spec Doc:
Updated paths:

Viewing API changes

For convenient view of the API changes made by this PR, refer to the URLs provided in the table
in the Generated ApiView comment added to this PR. You can use ApiView to show API versions diff.

Suppressing failures

If one or multiple validation error/warning suppression(s) is detected in your PR, please follow the
Swagger-Suppression-Process
to get approval.

❔Got questions? Need additional info?? We are here to help!

Contact us!

The Azure API Review Board is dedicated to helping you create amazing APIs. You can read about our mission and learn more about our process on our wiki.

💬 Teams Channel
💌 email

Click here for links to tools, specs, guidelines & other good stuff

Tooling

Open API validation tools were run on this PR. Go here to see how to fix errors
Spectral Linting

Guidelines & Specifications

Helpful Links

Schedule a data plane REST API spec review

Getting help

First, please carefully read through this PR description, from top to bottom.
If you don't have permissions to remove or add labels to the PR, request write access per aka.ms/azsdk/access#request-access-to-rest-api-or-sdk-repositories
To understand what you must do next to merge this PR, see the Next Steps to Merge comment. It will appear within few minutes of submitting this PR and will continue to be up-to-date with current PR state.
For guidance on fixing this PR CI check failures, see the hyperlinks provided in given failure
and https://aka.ms/ci-fix.
If the PR CI checks appear to be stuck in queued state, please add a comment with contents /azp run.
This should result in a new comment denoting a PR validation pipeline has started and the checks should be updated after few minutes.
If the help provided by the previous points is not enough, post to https://aka.ms/azsdk/support/specreview-channel and link to this PR.

openapi-pipeline-app · 2025-01-06T03:16:08Z

Next Steps to Merge

Next steps that must be taken to merge this PR:

❌ This is the public specs repo main branch which is not intended for iterative development.
You must acknowledge that you understand that after this PR is merged, you won't be able to stop your changes from being published to Azure customers.
If this is what you intend, add PublishToCustomers label to your PR.
Otherwise, retarget this PR onto a feature branch, i.e. with prefix release- (see aka.ms/azsdk/api-versions#release--branches).
❌ This PR has at least one change violating Azure versioning policy (label: VersioningReviewRequired).
To unblock this PR, either a) introduce a new API version with these changes instead of modifying an existing API version, or b) follow the process at aka.ms/brch.
❌ The required check named Swagger PrettierCheck has failed. Refer to the check in the PR's 'Checks' tab for details on how to fix it and consult the aka.ms/ci-fix guide

openapi-pipeline-app · 2025-01-06T03:16:09Z

Generated ApiView

Language	Package Name	ApiView Link
JavaScript	@azure-rest/microsoft-app-dynamicsessions-rest	https://apiview.dev/Assemblies/Review/271a97363c604130921a89db618ac151?revisionId=a6ca5dde1a3340a48511925ee534720f
TypeSpec	Microsoft.App.DynamicSessions	https://apiview.dev/Assemblies/Review/0df869ba4f0a43928e7a1e8561f20ee2?revisionId=7f6039071f9e4019b593ec6d9b55dca5

microsoft-github-policy-service · 2025-01-06T03:16:26Z

Thank you for your contribution shivamkm07! We will review the pull request and get back to you soon.

azure-sdk · 2025-01-06T03:18:50Z

API change check

APIView has identified API level changes in this PR and created following API reviews.

Microsoft.App.DynamicSessions

azure-sdk · 2025-01-06T03:18:50Z

API change check

APIView has identified API level changes in this PR and created following API reviews.

Microsoft.App.DynamicSessions

...fication/app/Microsoft.App.DynamicSessions/examples/2024-10-02-preview/ListSessions_Get.json

...ion/app/Microsoft.App.DynamicSessions/examples/2024-10-02-preview/SessionManagement_Get.json

...n/app/Microsoft.App.DynamicSessions/examples/2024-10-02-preview/SessionPoolMetadata_Get.json

specification/app/Microsoft.App.DynamicSessions/models/session.tsp

BlackRider97 · 2025-01-07T08:48:05Z

specification/app/Microsoft.App.DynamicSessions/routes.tsp

+  @get
+  get is Foundations.Operation<SessionIdentifier, Session>;
+
+  #suppress "@azure-tools/typespec-azure-core/use-standard-operations" "No suitable standard operation found."


Can't we avoid these suppression as it means they are not following standard pattern ?

Yeah I tried to use resource Operations which is the standard, but that requires resource Id i.e sessionIdentifier added in the URL Path instead of query params, which is not what we follow for other data-plane APIs. So these suppressions are required.

Signed-off-by: Shivam Kumar <[email protected]>

IshitaAsthana · 2025-01-21T08:23:49Z

specification/app/Microsoft.App.DynamicSessions/routes.tsp

+
+@route("/listSessions")
+interface ListSessions {
+  #suppress "@azure-tools/typespec-azure-core/use-standard-operations" "No suitable standard operation found."


it would be helpful to add the reason of suppression in comments

bhanuprakash-1 · 2025-01-21T09:58:56Z

...app/data-plane/Microsoft.App.DynamicSessions/preview/2024-10-02-preview/DynamicSessions.json

+            "$ref": "#/parameters/Azure.Core.Foundations.ApiVersionParameter"
+          },
+          {
+            "name": "offset",


Is this word "offset" the correct word to be used according to Azure API guidelines? Did you see this "offset" word recommended somewhere or did you comeup with it?

bhanuprakash-1 · 2025-01-21T10:00:24Z

...app/data-plane/Microsoft.App.DynamicSessions/preview/2024-10-02-preview/DynamicSessions.json

+            "format": "int32"
+          },
+          {
+            "name": "limit",


Same comment for this word "limit" as well. From what I remember there are recommended words to use. Can you check once incase you haven't checked?

The above comments are just suggestions, you can ignore them unless API review team asks you to update.

Anyways, according to the guidelines I see that recommended words are "maxpagesize" instead of "limit" and "skip" instead of "offset" words.

The guidelines are here: https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#query-options

bhanuprakash-1 · 2025-01-21T10:08:34Z

...app/data-plane/Microsoft.App.DynamicSessions/preview/2024-10-02-preview/DynamicSessions.json

+        "sessions": {
+          "type": "array",
+          "description": "The list of sessions.",
+          "items": {


Just a suggestion, ignore if not relevant: Do we want to send "nextLink" field in the response body with an absolute URL that the client can GET in order to retrieve the next page of the collection.

https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#query-options:~:text=DO%20return%20a%20nextLink%20field%20with

mikekistler

The PR pipeline tooling is failing because of a bad example file that is already merged to main:

CodeExecution_Execute_Async_Running.json

The problem is that the headers field is not allowed here. It should be included in the parameters section.

A quick check of the other example files found a second occurrence of this problem in

CodeExecution_Execute_Async_Completed.json

Please open a separate PR to correct this problem (fixing just these files)

The runtime failure in the PR tooling obscures a deviation from the Azure Versioning policy. The 2024-10-02-preview API version was already merged to main in the public repo, so updates to this version must be done in a new API version. I've added the VersioningReviewRequired label by hand to signal this.

Adding new Session Data Plane APIs

1e03a68

shivamkm07 requested review from jijohn14 and Juliehzl as code owners January 6, 2025 03:16

shivamkm07 changed the title ~~Adding new Session Data Plane APIs~~ Adding new ACA Session Data Plane APIs Jan 6, 2025

microsoft-github-policy-service bot added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Jan 6, 2025

AzureRestAPISpecReview added data-plane TypeSpec Authored with TypeSpec labels Jan 6, 2025

BlackRider97 reviewed Jan 7, 2025

View reviewed changes

Shivam Kumar and others added 5 commits January 16, 2025 04:28

Review comments

2d6c6b8

Signed-off-by: Shivam Kumar <[email protected]>

Adding parity between get and list

7345742

Signed-off-by: Shivam Kumar <[email protected]>

Adding pagination to listSessions api

690b487

Signed-off-by: Shivam Kumar <[email protected]>

updating examples

8ca1673

Signed-off-by: Shivam Kumar <[email protected]>

Merge branch 'main' into add-capps-session-metadata-apis

37cfe0f

IshitaAsthana approved these changes Jan 21, 2025

View reviewed changes

bhanuprakash-1 approved these changes Jan 21, 2025

View reviewed changes

mikekistler linked an issue Jan 23, 2025 that may be closed by this pull request

[Azure Container Apps - Azure Container Apps] API Review #31961

Open

mikekistler mentioned this pull request Jan 23, 2025

[Azure Container Apps - Azure Container Apps] API Review #31961

Open

mikekistler requested changes Jan 23, 2025

View reviewed changes

mikekistler added the VersioningReviewRequired <valid label in PR review process>add this label when versioning review is required label Jan 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding new ACA Session Data Plane APIs #31996

Adding new ACA Session Data Plane APIs #31996

shivamkm07 commented Jan 6, 2025 •

edited

Loading

openapi-pipeline-app bot commented Jan 6, 2025 •

edited

Loading

openapi-pipeline-app bot commented Jan 6, 2025 •

edited

Loading

microsoft-github-policy-service bot commented Jan 6, 2025

azure-sdk commented Jan 6, 2025

azure-sdk commented Jan 6, 2025 •

edited

Loading

BlackRider97 Jan 7, 2025

shivamkm07 Jan 16, 2025

IshitaAsthana Jan 21, 2025

bhanuprakash-1 Jan 21, 2025

bhanuprakash-1 Jan 21, 2025

bhanuprakash-1 Jan 21, 2025

bhanuprakash-1 Jan 21, 2025

bhanuprakash-1 Jan 21, 2025

mikekistler left a comment

Adding new ACA Session Data Plane APIs #31996

Are you sure you want to change the base?

Adding new ACA Session Data Plane APIs #31996

Conversation

shivamkm07 commented Jan 6, 2025 • edited Loading

Data Plane API Specification Update Pull Request

PR review workflow diagram

API Info: The Basics

Change Scope

Viewing API changes

Suppressing failures

❔Got questions? Need additional info?? We are here to help!

Tooling

Guidelines & Specifications

Helpful Links

Getting help

openapi-pipeline-app bot commented Jan 6, 2025 • edited Loading

Next Steps to Merge

openapi-pipeline-app bot commented Jan 6, 2025 • edited Loading

Generated ApiView

microsoft-github-policy-service bot commented Jan 6, 2025

azure-sdk commented Jan 6, 2025

azure-sdk commented Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikekistler left a comment

Choose a reason for hiding this comment

shivamkm07 commented Jan 6, 2025 •

edited

Loading

openapi-pipeline-app bot commented Jan 6, 2025 •

edited

Loading

openapi-pipeline-app bot commented Jan 6, 2025 •

edited

Loading

azure-sdk commented Jan 6, 2025 •

edited

Loading