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

Update code to support adding and parsing provider name #1032

Merged

Conversation

@bquantump
Copy link
Member Author

Pavel Krymets FTE 1 hour ago Member
Typically we don't modify the CodeModel during the generation. If some logic needs to be applied to the output model it needs to go into the corresponding output model class (https://github.com/Azure/autorest.csharp/blob/feature/v3/src/AutoRest.CSharp/Output/Models/Client.cs , https://github.com/Azure/autorest.csharp/blob/feature/v3/src/AutoRest.CSharp/Output/Models/RestClient.cs , https://github.com/Azure/autorest.csharp/blob/feature/v3/src/AutoRest.CSharp/Output/Models/Types/ObjectType.cs , etc.)

@bquantump
Copy link
Member Author

#1031 (review)

@bquantump
Copy link
Member Author

Pavel Krymets FTE 1 hour ago Member
Typically we don't modify the CodeModel during the generation. If some logic needs to be applied to the output model it needs to go into the corresponding output model class (https://github.com/Azure/autorest.csharp/blob/feature/v3/src/AutoRest.CSharp/Output/Models/Client.cs , https://github.com/Azure/autorest.csharp/blob/feature/v3/src/AutoRest.CSharp/Output/Models/RestClient.cs , https://github.com/Azure/autorest.csharp/blob/feature/v3/src/AutoRest.CSharp/Output/Models/Types/ObjectType.cs , etc.)

@pakrym This property will be needed by more than one output type for us (a new client and model output we will make). Is there any other good place to put this code given that?

@pakrym
Copy link
Contributor

pakrym commented Feb 16, 2021

Can you please add a description of what this feature would ultimately achieve? It would help figure out a good place for this to go. The other option might be to integrate this feature into some existing code to see it work.

@bquantump
Copy link
Member Author

Can you please add a description of what this feature would ultimately achieve? It would help figure out a good place for this to go. The other option might be to integrate this feature into some existing code to see it work.

This feature is to support basically having a property somewhere (I put it in the code model for this PR) that has the full provider name for a give resource (1:1 with an operation group). For example, subnet would be Microsoft.Network/virtualNetworks/subnets. We can either get this from the swagger or the OperationsGroupMapping list in the configuration file also added in this PR.

This property will need to be used to generate multiple new output types. (our new model and client types). These types are not implemented in code yet and will be implemented in later PRs.

@pakrym
Copy link
Contributor

pakrym commented Feb 16, 2021

How do we know this code works?

https://github.com/Azure/autorest.csharp/blob/feature/v3/src/AutoRest.CSharp/Output/Builders/BuilderHelpers.cs might be a reasonable place for this.

}
return providerName.TrimEnd('/');
}
private static List<string> GetPathSegments(string httpRequestUri)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can

public static IEnumerable<(string Text, bool IsLiteral)> GetPathParts(string? path)
be used here?

@bquantump
Copy link
Member Author

How do we know this code works?

https://github.com/Azure/autorest.csharp/blob/feature/v3/src/AutoRest.CSharp/Output/Builders/BuilderHelpers.cs might be a reasonable place for this.

These are all static and we would not want to run this logic to parse the URI to get the provider name more than once, since the logic is not simple. That is why we want to really store somewhere that many output types can use.

@pakrym
Copy link
Contributor

pakrym commented Feb 16, 2021

How do we know this code works?

These are all static and we would not want to run this logic to parse the URI to get the provider name more than once, since the logic is not simple

I doubt it would noticeably affect the generation time. We can cache it if perf becomes a problem and we would have a better feel of what other moving pieces are.

src/AutoRest.CSharp/Input/CodeModelPartials.cs Outdated Show resolved Hide resolved
src/AutoRest.CSharp/Input/CodeModelPartials.cs Outdated Show resolved Hide resolved
src/AutoRest.CSharp/Output/Models/Types/OutputLibrary.cs Outdated Show resolved Hide resolved
src/AutoRest.CSharp/Output/Models/Types/OutputLibrary.cs Outdated Show resolved Hide resolved
src/AutoRest.CSharp/Output/Models/Types/OutputLibrary.cs Outdated Show resolved Hide resolved
src/AutoRest.CSharp/Output/Models/Types/OutputLibrary.cs Outdated Show resolved Hide resolved
src/AutoRest.CSharp/Output/Models/Types/OutputLibrary.cs Outdated Show resolved Hide resolved
src/AutoRest.CSharp/Output/Models/Types/OutputLibrary.cs Outdated Show resolved Hide resolved
@bquantump bquantump requested a review from m-nash February 18, 2021 01:18
@bquantump
Copy link
Member Author

This was referenced Feb 18, 2021
@pakrym
Copy link
Contributor

pakrym commented Feb 18, 2021

Why is this Azure/azure-rest-api-specs@d9cf7c7/specification/imds/data-plane/readme.md
have azure-arm: true
when it is data-pane?

No idea.

@bquantump bquantump merged commit 94d4455 into Azure:feature/mgmt-track2 Feb 18, 2021
m-nash added a commit that referenced this pull request Apr 1, 2021
* Mnash 5154 skeleton operations (#1011)

* create shell resource operations objects

* added trailing newline

* Revert "added trailing newline"

This reverts commit a24d17d.

* update launch settings to add a compute option vs modifying

* updates after adding dummy constructor

* updates after merge

* updates to generated code

* address pr comments

* update after redoing npm install

* update launchsettings

* update samples list

* revert unintended change

* update ordering for launch settings

* add resource container shell to mgmt generation (#1025)

* add resource container shell to mgmt generation

* Address review comments

* Renaming Azure.ResourceManager.Compute to Azure.ResourceManager.Sample (#1034)

* Update code to support adding and parsing provider name (#1032)

* WIP: Update code for provider name

* Add operationsGroupMappings example

* Only Write out if has field

* WIP: update operations config


* Update readme

* Wip: update compute


* WIP: off by one

* Remove Synapse

* Remove more tests

* Remove all mgmt test

* Update OutputLibrary.cs

* remove imds for now

* Update Operations

* Tenant id (#1037)

* WIP: tenant id

* WIP: rebase

* Rebase

* WIP

* WIP: tenant deco

* NIT

* NIT

* WIP: Update

* NIT newline

* Updates

* WIP: updates

* Updates

* NIT

* Enable OperationGroup get associated resource name from one property (#1049)

* WIP: update (#1052)

* WIP: update

* Updates

* Fix bad merge

* White space

* Update config

* Mergev3tomgmt (#1062)

* Generate public client constructors and options (#1046)

Generate public client constructors and options

* Make client options class partial (#1055)

* Disable buffering for stream responses (#1060)

* updates after merge

* duplicate end file

* regen after merge

Co-authored-by: ShivangiReja <45216704+ShivangiReja@users.noreply.github.com>
Co-authored-by: Pavel Krymets <pavel@krymets.com>

* merge feature/v3 into feature/mgmt-track2 (#1072)

* Generate public client constructors and options (#1046)

Generate public client constructors and options

* Make client options class partial (#1055)

* Disable buffering for stream responses (#1060)

* Add Url Encoded Body support (#1042)

- Fixes: #986

* Fix LRO create request from relative path on Unix (#1059)

* Remove automated buffering logic (#1064)

* Add System.Text.Json converter to models (#1063)

Fixes: #947

* Fix feature/v3 integration with azure-sdk-for-net (#1065)

- Autorest core 3.0.6372 and above crash due to directive ordering bug Tim is digging into now
- New shared code does not compile cleanly with azure-sdk-for-net rules

* Hide paging classes (#1066)

* Add help content (#1067)

* Add help content

* update after merge

Co-authored-by: ShivangiReja <45216704+ShivangiReja@users.noreply.github.com>
Co-authored-by: Pavel Krymets <pavel@krymets.com>
Co-authored-by: Chris Hamons <chris.hamons@microsoft.com>
Co-authored-by: pavelprystinka <30868871+pavelprystinka@users.noreply.github.com>
Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>

* Support generating empty data and data serialization file (#1074)

* Add compute to test projects

* Support generating empty [Resource]Data.cs and [Resource]Data.Serialization.cs

* Upload compute codegen result

* Remove codegen result

* Resolve merge conflicts

* Resolve reference errors

* Refactor code

* Regenerate Azure.ResourceManager.Sample

* Regenerate Azure.Management.Storage

* Update all other codegen files when running Generate.ps1

* Re-run Generate.ps1

* Remove the stale compute project

* Decorate schema

* Change ResourceData to inherit from ObjectType

* Regenerate Azure.Management.Storage

* Regenerate Azure.ResourceManager.Sample

* Correct the Resource name in [Resource]Container.cs and [Resource]Operation.cs

* Regenerate Azure.Management.Storage

* Regenerate Azure.ResourceManager.Sample

* Cleanup comments

* Re-run Generate.ps1

* Address review comments

* Run Generate.ps1

* regen

* add files

Co-authored-by: YalinLi0312 <yall@microsoft.com>

* [Mgmt Track 2] 5156 generate empty resource autorest change (#1076)

* Empty Resource Code Change

* CI fix

* Empty Resource Sample Update

* Rename arm to mgmt as ARM is ingored

* Updating storage sample

* Updated resource to be in the Generated folder

* Updates after merge

* regenerate with feature/v3 changes

* Update [Resource]Data to inherit the exact matching base class (#1095)

* Add Azure.ResourceManager.Core to package reference group

* Override inheritance in [Resource]Data

* Regenerate Azure.Resourcemanager.Sample

* Regenerate Azure.Management.Storage

* Update Azure.Core and Azure.ResourceManager.Core version

* Rebuild model inheritance

* Correct the properties to generate

* Update ResourceDataWriter and its serialization writer

* Regenerate Azure.ResourceManager.Sample

* Update Azure.resourceManager.Core version

* Regenerate Azure.Management.Storage

* Run Generate.ps1

* Address review comments

* Run Generate.ps1

* initial changes to compile

* finish merge

* initial mgmt fork

* more changes to mgmt fork

* updates after merge

* remove unnecessary Arm configuration checks

* missed a merge conflict to enable the inheritance code

* auto-generate after merge and changes

* address review comments

* change to ireadonlydictionary

* Remove mgmt response headers

* LROs

* remove M-LROs

* remove files until next pr

* address pr comments

* update csproj writer

* update protected property

Co-authored-by: Allen Zhang <allenzhang@live.com>
Co-authored-by: bquantump <53361486+bquantump@users.noreply.github.com>
Co-authored-by: Yalin Li <yall@microsoft.com>
Co-authored-by: ShivangiReja <45216704+ShivangiReja@users.noreply.github.com>
Co-authored-by: Pavel Krymets <pavel@krymets.com>
Co-authored-by: Chris Hamons <chris.hamons@microsoft.com>
Co-authored-by: pavelprystinka <30868871+pavelprystinka@users.noreply.github.com>
Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>
Co-authored-by: m-nash <prognash@microsoft.com>
m-nash added a commit that referenced this pull request Apr 6, 2021
* Mnash 5154 skeleton operations (#1011)

* create shell resource operations objects

* added trailing newline

* Revert "added trailing newline"

This reverts commit a24d17d.

* update launch settings to add a compute option vs modifying

* updates after adding dummy constructor

* updates after merge

* updates to generated code

* address pr comments

* update after redoing npm install

* update launchsettings

* update samples list

* revert unintended change

* update ordering for launch settings

* add resource container shell to mgmt generation (#1025)

* add resource container shell to mgmt generation

* Address review comments

* Renaming Azure.ResourceManager.Compute to Azure.ResourceManager.Sample (#1034)

* Update code to support adding and parsing provider name (#1032)

* WIP: Update code for provider name

* Add operationsGroupMappings example

* Only Write out if has field

* WIP: update operations config


* Update readme

* Wip: update compute


* WIP: off by one

* Remove Synapse

* Remove more tests

* Remove all mgmt test

* Update OutputLibrary.cs

* remove imds for now

* Update Operations

* Tenant id (#1037)

* WIP: tenant id

* WIP: rebase

* Rebase

* WIP

* WIP: tenant deco

* NIT

* NIT

* WIP: Update

* NIT newline

* Updates

* WIP: updates

* Updates

* NIT

* Enable OperationGroup get associated resource name from one property (#1049)

* WIP: update (#1052)

* WIP: update

* Updates

* Fix bad merge

* White space

* Update config

* Mergev3tomgmt (#1062)

* Generate public client constructors and options (#1046)

Generate public client constructors and options

* Make client options class partial (#1055)

* Disable buffering for stream responses (#1060)

* updates after merge

* duplicate end file

* regen after merge

Co-authored-by: ShivangiReja <45216704+ShivangiReja@users.noreply.github.com>
Co-authored-by: Pavel Krymets <pavel@krymets.com>

* merge feature/v3 into feature/mgmt-track2 (#1072)

* Generate public client constructors and options (#1046)

Generate public client constructors and options

* Make client options class partial (#1055)

* Disable buffering for stream responses (#1060)

* Add Url Encoded Body support (#1042)

- Fixes: #986

* Fix LRO create request from relative path on Unix (#1059)

* Remove automated buffering logic (#1064)

* Add System.Text.Json converter to models (#1063)

Fixes: #947

* Fix feature/v3 integration with azure-sdk-for-net (#1065)

- Autorest core 3.0.6372 and above crash due to directive ordering bug Tim is digging into now
- New shared code does not compile cleanly with azure-sdk-for-net rules

* Hide paging classes (#1066)

* Add help content (#1067)

* Add help content

* update after merge

Co-authored-by: ShivangiReja <45216704+ShivangiReja@users.noreply.github.com>
Co-authored-by: Pavel Krymets <pavel@krymets.com>
Co-authored-by: Chris Hamons <chris.hamons@microsoft.com>
Co-authored-by: pavelprystinka <30868871+pavelprystinka@users.noreply.github.com>
Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>

* Support generating empty data and data serialization file (#1074)

* Add compute to test projects

* Support generating empty [Resource]Data.cs and [Resource]Data.Serialization.cs

* Upload compute codegen result

* Remove codegen result

* Resolve merge conflicts

* Resolve reference errors

* Refactor code

* Regenerate Azure.ResourceManager.Sample

* Regenerate Azure.Management.Storage

* Update all other codegen files when running Generate.ps1

* Re-run Generate.ps1

* Remove the stale compute project

* Decorate schema

* Change ResourceData to inherit from ObjectType

* Regenerate Azure.Management.Storage

* Regenerate Azure.ResourceManager.Sample

* Correct the Resource name in [Resource]Container.cs and [Resource]Operation.cs

* Regenerate Azure.Management.Storage

* Regenerate Azure.ResourceManager.Sample

* Cleanup comments

* Re-run Generate.ps1

* Address review comments

* Run Generate.ps1

* regen

* add files

Co-authored-by: YalinLi0312 <yall@microsoft.com>

* [Mgmt Track 2] 5156 generate empty resource autorest change (#1076)

* Empty Resource Code Change

* CI fix

* Empty Resource Sample Update

* Rename arm to mgmt as ARM is ingored

* Updating storage sample

* Updated resource to be in the Generated folder

* Updates after merge

* regenerate with feature/v3 changes

* WIP: First cust parent algo

* Update OutputLibrary.cs

* WIP

* WIP:

* WIP

* WIP

* WIP

* WIP: Updates

* WIP

* WIP:

* WIP

* WIP

* WIP

* WIP

* WIP: update configs

* WIP: update test

* WIP

* Update [Resource]Data to inherit the exact matching base class (#1095)

* Add Azure.ResourceManager.Core to package reference group

* Override inheritance in [Resource]Data

* Regenerate Azure.Resourcemanager.Sample

* Regenerate Azure.Management.Storage

* Update Azure.Core and Azure.ResourceManager.Core version

* Rebuild model inheritance

* Correct the properties to generate

* Update ResourceDataWriter and its serialization writer

* Regenerate Azure.ResourceManager.Sample

* Update Azure.resourceManager.Core version

* Regenerate Azure.Management.Storage

* Run Generate.ps1

* Address review comments

* Run Generate.ps1

* initial changes to compile

* finish merge

* initial mgmt fork

* Updates test

* more changes to mgmt fork

* updates after merge

* remove unnecessary Arm configuration checks

* missed a merge conflict to enable the inheritance code

* auto-generate after merge and changes

* address review comments

* Updates

* Update

* WIP

* Merge

* Update

* Updates

* Updates

* Update DataPlaneResponseHeaderGroupWriter.cs

* Update DataPlaneLongRunningOperationWriter.cs

* Update DataPlaneLongRunningOperationMethod.cs

* address review comments

* add resource rename test

Co-authored-by: m-nash <64171366+m-nash@users.noreply.github.com>
Co-authored-by: Allen Zhang <allenzhang@live.com>
Co-authored-by: Yalin Li <yall@microsoft.com>
Co-authored-by: ShivangiReja <45216704+ShivangiReja@users.noreply.github.com>
Co-authored-by: Pavel Krymets <pavel@krymets.com>
Co-authored-by: Chris Hamons <chris.hamons@microsoft.com>
Co-authored-by: pavelprystinka <30868871+pavelprystinka@users.noreply.github.com>
Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>
Co-authored-by: m-nash <prognash@microsoft.com>
JonathanCrd added a commit that referenced this pull request Apr 9, 2021
* Mnash 5154 skeleton operations (#1011)

* create shell resource operations objects

* added trailing newline

* Revert "added trailing newline"

This reverts commit a24d17d.

* update launch settings to add a compute option vs modifying

* updates after adding dummy constructor

* updates after merge

* updates to generated code

* address pr comments

* update after redoing npm install

* update launchsettings

* update samples list

* revert unintended change

* update ordering for launch settings

* add resource container shell to mgmt generation (#1025)

* add resource container shell to mgmt generation

* Address review comments

* Renaming Azure.ResourceManager.Compute to Azure.ResourceManager.Sample (#1034)

* Update code to support adding and parsing provider name (#1032)

* WIP: Update code for provider name

* Add operationsGroupMappings example

* Only Write out if has field

* WIP: update operations config


* Update readme

* Wip: update compute


* WIP: off by one

* Remove Synapse

* Remove more tests

* Remove all mgmt test

* Update OutputLibrary.cs

* remove imds for now

* Update Operations

* Tenant id (#1037)

* WIP: tenant id

* WIP: rebase

* Rebase

* WIP

* WIP: tenant deco

* NIT

* NIT

* WIP: Update

* NIT newline

* Updates

* WIP: updates

* Updates

* NIT

* Enable OperationGroup get associated resource name from one property (#1049)

* WIP: update (#1052)

* WIP: update

* Updates

* Fix bad merge

* White space

* Update config

* Mergev3tomgmt (#1062)

* Generate public client constructors and options (#1046)

Generate public client constructors and options

* Make client options class partial (#1055)

* Disable buffering for stream responses (#1060)

* updates after merge

* duplicate end file

* regen after merge

Co-authored-by: ShivangiReja <45216704+ShivangiReja@users.noreply.github.com>
Co-authored-by: Pavel Krymets <pavel@krymets.com>

* merge feature/v3 into feature/mgmt-track2 (#1072)

* Generate public client constructors and options (#1046)

Generate public client constructors and options

* Make client options class partial (#1055)

* Disable buffering for stream responses (#1060)

* Add Url Encoded Body support (#1042)

- Fixes: #986

* Fix LRO create request from relative path on Unix (#1059)

* Remove automated buffering logic (#1064)

* Add System.Text.Json converter to models (#1063)

Fixes: #947

* Fix feature/v3 integration with azure-sdk-for-net (#1065)

- Autorest core 3.0.6372 and above crash due to directive ordering bug Tim is digging into now
- New shared code does not compile cleanly with azure-sdk-for-net rules

* Hide paging classes (#1066)

* Add help content (#1067)

* Add help content

* update after merge

Co-authored-by: ShivangiReja <45216704+ShivangiReja@users.noreply.github.com>
Co-authored-by: Pavel Krymets <pavel@krymets.com>
Co-authored-by: Chris Hamons <chris.hamons@microsoft.com>
Co-authored-by: pavelprystinka <30868871+pavelprystinka@users.noreply.github.com>
Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>

* Support generating empty data and data serialization file (#1074)

* Add compute to test projects

* Support generating empty [Resource]Data.cs and [Resource]Data.Serialization.cs

* Upload compute codegen result

* Remove codegen result

* Resolve merge conflicts

* Resolve reference errors

* Refactor code

* Regenerate Azure.ResourceManager.Sample

* Regenerate Azure.Management.Storage

* Update all other codegen files when running Generate.ps1

* Re-run Generate.ps1

* Remove the stale compute project

* Decorate schema

* Change ResourceData to inherit from ObjectType

* Regenerate Azure.Management.Storage

* Regenerate Azure.ResourceManager.Sample

* Correct the Resource name in [Resource]Container.cs and [Resource]Operation.cs

* Regenerate Azure.Management.Storage

* Regenerate Azure.ResourceManager.Sample

* Cleanup comments

* Re-run Generate.ps1

* Address review comments

* Run Generate.ps1

* regen

* add files

Co-authored-by: YalinLi0312 <yall@microsoft.com>

* GenCode changes

* Generated files with ListAvailableLocations

* [Mgmt Track 2] 5156 generate empty resource autorest change (#1076)

* Empty Resource Code Change

* CI fix

* Empty Resource Sample Update

* Rename arm to mgmt as ARM is ingored

* Updating storage sample

* Updated resource to be in the Generated folder

* Updates after merge

* NuGet Packages added

* First iteration

* Create ResourceOperationsTests.cs

* Add ResourceType property

* ResourceType

* regenerate with feature/v3 changes

* wip changes

* update to handle overrides

* Update Azure.ResourceManager.Core Version

* CancellationToken in Get()

* Azure.ResourceManager.Core version

* Update Azure.ResourceManager.Core version

* Style corrections in ResourceOperation Writer

* Update OutputLibrary.cs

* Temporarily remove Test Case

* Test cases for *Operations types

* Test files re-generated

* revert merge

* manual updates after merge

* fix issue with merge

* regen after merge

* move inheritance chooser

* regen after merge

* regen after merge

* regen after merge

* Fixing comments

* Re-generate Azure.ResourceManager.Sample

* Re-run Generate.ps1

* Default constructor for TestProjectTests

* Missing using statement

* Addressing pakrym comments

* Re-generated files up to date

Co-authored-by: m-nash <64171366+m-nash@users.noreply.github.com>
Co-authored-by: Allen Zhang <allenzhang@live.com>
Co-authored-by: bquantump <53361486+bquantump@users.noreply.github.com>
Co-authored-by: Yalin Li <yall@microsoft.com>
Co-authored-by: ShivangiReja <45216704+ShivangiReja@users.noreply.github.com>
Co-authored-by: Pavel Krymets <pavel@krymets.com>
Co-authored-by: Chris Hamons <chris.hamons@microsoft.com>
Co-authored-by: pavelprystinka <30868871+pavelprystinka@users.noreply.github.com>
Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>
Co-authored-by: m-nash <prognash@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants