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

[TCGC] Need a way to remove operation from TCGC #964

Open
4 tasks done
live1206 opened this issue Jun 6, 2024 · 10 comments · May be fixed by #1998
Open
4 tasks done

[TCGC] Need a way to remove operation from TCGC #964

live1206 opened this issue Jun 6, 2024 · 10 comments · May be fixed by #1998
Assignees
Labels
feature New feature or request lib:tcgc Issues for @azure-tools/typespec-client-generator-core library

Comments

@live1206
Copy link
Member

live1206 commented Jun 6, 2024

Clear and concise description of the problem

For .NET SDK, we have requirement to remove certain operations, such as Operations.list should always be removed as duplicated within resource manager.
Right now, the best approach we have is below, details

@armResourceOperations
interface Employees {
  get is ArmResourceRead<Employee>;
}

@@access(Employees.get, Access.internal, "csharp");

But this operation still exists, .NET SDK need to handle it specifically.
Is it possible to remove this operation from TCGC result? for instance,

@@access(Employees.get, Access.never, "csharp");

Or can we have a new decorator to remove an operation in TCGC?

Checklist

  • Follow our Code of Conduct
  • Check that this issue is about the Azure libraries for typespec. For feature request in the typespec language or core libraries file it in the TypeSpec repo
  • Read the docs.
  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.
@live1206
Copy link
Member Author

live1206 commented Jun 6, 2024

cc @tadelesh @m-nash @ArcturusZhang

@live1206 live1206 added the lib:tcgc Issues for @azure-tools/typespec-client-generator-core library label Jun 6, 2024
@live1206 live1206 changed the title [TCGC] Access [TCGC] Need a way to remove operation from TCGC Jun 6, 2024
@m-nash
Copy link
Member

m-nash commented Jun 10, 2024

The original design goal for this was we would have an @scope decorator which simply modified the scope of any given item.

I would imagine we could do @@scope(Service.op, "~csharp"). Meaning the operation applies to everything but csharp.

@tadelesh tadelesh added the feature New feature or request label Jun 11, 2024
@live1206
Copy link
Member Author

live1206 commented Jul 17, 2024

If we handle this in .NET generator directly, the usage calculation in TCGC will miss this part and end into inaccurate usage.
So, would like to still handle this in TCGC despite this requirement is only for .NET. Potentially, it could benefit the other languages.

Normally, a decorator could be applied to Type, to avoid the over complexity only apply it to Operation as needed now.

usage:

# "~" means exclude the target operation from the defined scope
@scope("~csharp")
op nameInService: void;

@scope("java")
op nameInService: void;

validation:

  • throw empty-scope since there is no point to define an empty scope
  • throw is-in-use if the operation itself is in use or the models/enums contained in the operation is in use, the scope change will potentially affect the use references
    • need to check if the operation is referenced in other places
    • need to check the contained models/enums are in use outside the operation
  • throw exclude-target-not-existed if the target to be excluded does not exist in main.tsp
  • throw include-target-existed if the target to be included already exists in main.tsp
  • throw include-target-not-declared if the target to be included is not declared, the main usage of inclusion is within client.tsp by adding some extra operation declaration

effect: the operation along with everything inside(we have validation to ensure everything inside is not referenced outside this operation) will be removed from the excluded language scopes, and the usage should be calculated correctly based on this.

@live1206
Copy link
Member Author

live1206 commented Jul 17, 2024

Found https://github.com/Azure/typespec-azure-pr/issues/2732 regarding scope of decorators in TCGC.
I think the proposal of @scope decorator is identical to @exclude and @include with scopes, just need to extend the entity from Model to Operation | Model.
And now @exclude and @include are deprecated, because we think they can be replaced by @usage and @access. So, don't feel like adding a new @scope decorator will be the approach we would take.

Another option raise from @ArcturusZhang is to use @convenientAPI, TCGC has taken care of the Model usage. .NET SDK mgmt generator just needs to omit the operation if InputOperation.GenerateConvenienceMethod is false, same behavior as DPG

@live1206
Copy link
Member Author

live1206 commented Jul 18, 2024

Found Azure/typespec-azure-pr#2732 regarding scope of decorators in TCGC. I think the proposal of @scope decorator is identical to @exclude and @include with scopes, just need to extend the entity from Model to Operation | Model. And now @exclude and @include are deprecated, because we think they can be replaced by @usage and @access. So, don't feel like adding a new @scope decorator will be the approach we would take.

Another option raise from @ArcturusZhang is to use @convenientAPI, TCGC has taken care of the Model usage. .NET SDK mgmt generator just needs to omit the operation if InputOperation.GenerateConvenienceMethod is false, same behavior as DPG

After clarification with @m-nash, the negation language scope is not implemented, but not abandoned.
The reason we need a new @scope decorator and it can't be replaced by the existing deprecated @include is because we can't define the behavior of @include or @exclude decorator with negation scope.

@iscai-msft iscai-msft added this to the [2024] September milestone Aug 7, 2024
@iscai-msft
Copy link
Contributor

@live1206 would you be able to contribute a design and implementation for this? Thanks!

@live1206
Copy link
Member Author

live1206 commented Aug 7, 2024

@live1206 would you be able to contribute a design and implementation for this? Thanks!

Sure, will add more details with what I have been doing.

@live1206
Copy link
Member Author

.NET SDK uses CrossLanguageDefinitionId to identify list_operations and exclude it in generator, this item is not blocking .NET SDK anymore, the priority decreases.

@markcowl markcowl removed this from the [2024] October milestone Oct 15, 2024
@live1206
Copy link
Member Author

Reopen since we still have the requirement in Azure/autorest.csharp#5191

@live1206 live1206 reopened this Dec 12, 2024
@ArthurMa1978
Copy link
Member

Please ensure that when an operation is removed, all related items, including models and samples, are also removed.

@live1206 live1206 linked a pull request Dec 18, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request lib:tcgc Issues for @azure-tools/typespec-client-generator-core library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants