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

Add legacy operations for multi-path scenario #2095

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

pshao25
Copy link
Member

@pshao25 pshao25 commented Jan 17, 2025

To proceed #234, add the legacy operation template into the library according to this design. It's almost the same as @markcowl 's design, except:

  1. Change the body name and doc for CustomPatchSync/CustomPatchAsync to align with those of ArmCustomPatchSync/ArmCustomPatchAsync.
  2. Change the DeleteAsync to DeleteWithoutOkAsync because its corresponding ArmResourceDeleteAsync is deprecated.

@microsoft-github-policy-service microsoft-github-policy-service bot added the lib:azure-resource-manager Issues for @azure-tools/typespec-azure-core library label Jan 17, 2025
@azure-sdk
Copy link
Collaborator

azure-sdk commented Jan 17, 2025

All changed packages have been documented.

  • @azure-tools/typespec-azure-resource-manager
Show changes

@azure-tools/typespec-azure-resource-manager - feature ✏️

Add legacy operations for multi-path scenario

@azure-sdk
Copy link
Collaborator

azure-sdk commented Jan 17, 2025

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs

Copy link
Member

@markcowl markcowl left a comment

Choose a reason for hiding this comment

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

I wonder if we want to export the types here as experimental until we have used them on a few more specs.

@pshao25
Copy link
Member Author

pshao25 commented Feb 7, 2025

I wonder if we want to export the types here as experimental until we have used them on a few more specs.

Isn't the design ready? What else we need to design more? If we don't get this in, every time when customer asks, we need to give them a copy of a legacy operation.

@AlitzelMendez
Copy link
Member

I wonder if we want to export the types here as experimental until we have used them on a few more specs.

do you mean like having an Azure.ResourceManager.Experimental?

@markcowl markcowl self-requested a review February 20, 2025 19:21

@doc("The content of the action request")
@bodyRoot
body: Request,
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to add two add an additional parameter for every one of these operations that has a request body, like:

RequestBody extends {} = {
  @doc("The content of the action request")
      @bodyRoot
      body: Request
},

This would allow optional request bodies, which are not standard, but occur in legacy apis

Copy link
Member

Choose a reason for hiding this comment

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

It also allows renaming the body parameter without decorators.

): Response | ErrorType;

/**
* @dev Delete a resource asynchronously
Copy link
Member

Choose a reason for hiding this comment

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

I'd like us to be explicit that these APIs are subject to change, and should only be used in converting legacy APIs to typespec.

If we could, I would consider an experimental types export like this: https://github.com/microsoft/typespec/blob/main/packages/events/package.json#L28

But we would have to validate that this works with imports and standard tsp compilation. The best current solution might be docs and either a decorator or rule that marks these types as volatile, and issues a warning if you use them (perhaps through conbining a decorator and linting rule, so that it could be globally suppressed in config).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib:azure-resource-manager Issues for @azure-tools/typespec-azure-core library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants