Merged
Conversation
Next Steps to Merge⌛ Please wait, next steps to merge this PR are being evaluated by automation. ⌛ |
Swagger Validation Report
|
| compared swaggers (via Oad v0.10.4)] | new version | base version |
|---|---|---|
| communicationservicejobrouter.json | 2023-11-01(a396918) | 2022-07-18-preview(main) |
The following breaking changes are detected by comparison with the latest preview version:
Only 30 items are listed, please refer to log for more details.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️LintDiff: 111 Warnings warning [Detail]
| compared tags (via openapi-validator v2.1.6) | new version | base version |
|---|---|---|
| package-jobrouter-2023-11-01 | package-jobrouter-2023-11-01(a396918) | default(main) |
[must fix]The following errors/warnings are introduced by current PR:
Only 30 items are listed, please refer to log for more details.
| Rule | Message | Related RPC [For API reviewers] |
|---|---|---|
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L37 |
||
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L85 |
||
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L93 |
||
| Schema with type: string has unrecognized format: eTag Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L104 |
||
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L138 |
||
| OperationId for patch method should contain both 'Create' and 'Update' Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L139 |
||
| 'PATCH' operation 'JobRouterAdministrationOperations_UpsertClassificationPolicy' should use method name 'Update'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L139 |
||
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L149 |
||
| Schema with type: string has unrecognized format: eTag Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L175 |
||
| Schema with type: string has unrecognized format: eTag Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L192 |
||
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L229 |
||
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L237 |
||
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L270 |
||
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L318 |
||
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L326 |
||
| Schema with type: string has unrecognized format: eTag Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L337 |
||
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L371 |
||
| OperationId for patch method should contain both 'Create' and 'Update' Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L372 |
||
| 'PATCH' operation 'JobRouterAdministrationOperations_UpsertDistributionPolicy' should use method name 'Update'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L372 |
||
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L382 |
||
| Schema with type: string has unrecognized format: eTag Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L408 |
||
| Schema with type: string has unrecognized format: eTag Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L425 |
||
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L462 |
||
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L470 |
||
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L503 |
||
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L551 |
||
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L559 |
||
| Schema with type: string has unrecognized format: eTag Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L570 |
||
| The summary and description values should not be same. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L604 |
||
| OperationId for patch method should contain both 'Create' and 'Update' Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L605 |
The following errors/warnings exist before current PR submission:
| Rule | Message |
|---|---|
| OperationId for patch method should contain both 'Create' and 'Update' Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L1885 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L2033 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L2084 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L2530 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L3908 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L4008 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L4184 |
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️❌SemanticValidation: 3 Errors, 0 Warnings failed [Detail]
| Rule | Message |
|---|---|
|
Discriminator must be a required property. JsonUrl: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L3824:33 |
|
Discriminator must be a required property. JsonUrl: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L2545:39 |
|
Discriminator must be a required property. JsonUrl: JobRouter/stable/2023-11-01/communicationservicejobrouter.json#L2869:38 |
️️✔️PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
️️✔️Automated merging requirements met succeeded [Detail] [Expand]
Swagger Generation Artifacts
|
Generated ApiView
|
Swagger Validation Report
|
Next Steps to Merge✔️ All automated merging requirements have been met! Refer to step 4 in the PR workflow diagram (even if your PR is for data plane, not ARM). |
mikekistler
reviewed
Nov 1, 2023
Member
mikekistler
left a comment
mikekistler
left a comment
There was a problem hiding this comment.
I scrubbed through your TypeSpec and left a bunch of questions.
sarkar-rajarshi
force-pushed
the
rsarkar/jobrouter/ga-version-typespec
branch
from
e1555a8 to
cad19d6
Compare
MaryGao
reviewed
Nov 3, 2023
specification/communication/Communication.JobRouter/tspconfig.yaml
Outdated
Show resolved
Hide resolved
Member
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
sarkar-rajarshi
requested review from
mikekistler and
scgbear
and removed request for
a team
Member
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Member
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
…p-code-customizations Rsarkar/jobrouter/ga/csharp code customizations
mikekistler
requested changes
Nov 8, 2023
Member
mikekistler
left a comment
mikekistler
left a comment
There was a problem hiding this comment.
Looking very good. I found one more round of minor improvements.
...tion/communication/data-plane/JobRouter/stable/2023-11-01/communicationservicejobrouter.json
Show resolved
Hide resolved
…mpty-models redefine empty model differently
…ss-review-feedback add defaults and change empty model representation
mikekistler
approved these changes
Nov 8, 2023
Member
mikekistler
left a comment
mikekistler
left a comment
There was a problem hiding this comment.
Looks good! 👍
Thanks for all the fixes! I found two more small things but I'll approve to avoid a re-review for these small changes.
Co-authored-by: Mike Kistler <mikekistler@microsoft.com>
Co-authored-by: Mike Kistler <mikekistler@microsoft.com>
Member
Author
|
@anuchandy this is ready to be merged |
|
Swagger pipeline restarted successfully, please wait for status update in this comment. |
MaryGao
reviewed
Nov 10, 2023
| partial-update: true | ||
| "@azure-tools/typespec-ts": | ||
| emitter-output-dir: "{js-sdk-folder}/sdk/{service-directory-name}/communication-job-router-rest" | ||
| package-dir: "communication-job-router" |
Member
There was a problem hiding this comment.
I don't think we should specify the emitter-output-dir here because we use the option package-dir. And it should be ${folder-name}-rest. See name convention here.
Could you update with below?
package-dir: "communication-job-router-rest"
zman-ms
pushed a commit
that referenced
this pull request
Jan 10, 2024
* jobrouter ga typespec initial commit * add x-ms-example * change unknown to object * fix consumes for upsert routes * fix naming validation error * remove x-ms-examples * unreferenced json file validation fix * change to using unknown * rename generated client to XXXXXRestClient * test generating convenience methods * add line * rebased * correctly represent uri * try out standard operations for classification policy * use standard ops wherever possible * try placing route decorator at namespace level * remove router decorator from namespace * update tspconfig * add conditional headers * use azure core etag * fix floats to doubles * update typespec-python options + regenerate rest specs * fix lint errors * add examples * ci fixes 2 * ci fixes 3 * remove reference to Azure.Core.Etag (throws lint error) * review feedback * added list query params * try codegen without url scalar * retry python emitter with url fix * try specifying python output directory * python emitter - change package-mode to azure-dataplane * remove @example decorator * remove redundant imports * fix examples * make ScheduleAndSuspendMode.ScheduleAt a required field * uri scalar add * clean up * Update JS sdk config to azure-rest * change JobMatchingMode polymorphic type * rename client remove Rest * rename acceptJob, declineJob to acceptJobOffer, declineJobOffer * test internal decorator * make all upsert methods internal * specify python package version * Make upsert methods internal * make status selectors internal for python * sdk feedback review items * more review feedback items * update examples * Add partial-update for java * try updating client.tsp to rename method for .NET * make ExceptionAction.Id optional * run prettier * fix more examples * minor doc update * Update projected names and internal modifier * Modify internal to only apply for java, csharp, python * Update internal * make upsert method visible for python * modify enums for python * revert * sdk review comment 3 * Test generator without endpoint * Revert change * manually add empty response schemas in swagger because of #25605 (comment) * add linter in tspconfig.yaml * add to custom words * doc updates * fix nextlink lint error * test changes in client.tsp for validation failures * run cleanup * Update java internal models * Remove projectedName annotations * more linter fixes * exception action make id not readonly since needs to be serialized * make methods for canceljon, complete job, close job and decline offer internal so as to not expose empty response objects * update azure-tools/typespec-ts emitter options * try update example for LintDiff errors * set nextLink value to null * Update unassignJob internal csharp * move out assignmentId to path * linter fixes * Revert unassign job changes * Add internal for unassign csharp * use standard ResourceAction for all action except closeJob * fix linters * close job should always return 200 * fix linter errors * Suppress violation ValidFormats in LintDiff * test method rename * try rename again * try again * try again * try again * set default values for min and maz concurrent offer * set default value of bypass selectors * try codegen removing @route decorator * clean up @route decorator * revert making unassign internal for csharp * try renaming completeJob to completeJobInternal for csharp * rename op cancel, close, complete, decline for csharp * Add "where" clause to ValidFormats suppression * redefine empty model differently * fix spelling of primitive * Suppress INVALID_TYPE caused by bug in ModelValidation - #25381 * add default and change empty model representation * Make unassign job internal * linter fixes * Update specification/communication/Communication.JobRouter/models.tsp Co-authored-by: Mike Kistler <mikekistler@microsoft.com> * Update specification/communication/Communication.JobRouter/models.tsp Co-authored-by: Mike Kistler <mikekistler@microsoft.com> * linter fixes --------- Co-authored-by: Adam Tuck <adamtuck@microsoft.com> Co-authored-by: Marc Ma <marcma@microsoft.com> Co-authored-by: Charandeep Parisineti <cparisineti@microsoft.com> Co-authored-by: williamzhao87 <williamzhao87@gmail.com> Co-authored-by: Mike Harder <mharder@microsoft.com> Co-authored-by: Mike Kistler <mikekistler@microsoft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Data Plane API - Pull Request
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.
Is this review for (select one):
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.
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 ApiViewcomment 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.
Click here for links to tools, specs, guidelines & other good stuff
Tooling
Guidelines & Specifications
Helpful Links