azure-sdk-for-net
azure-sdk-for-net copied to clipboard
Initialize Azure csharp plugin
resolves https://github.com/Azure/azure-sdk-for-net/issues/45616 resolves https://github.com/Azure/azure-sdk-for-net/issues/45619 resolves https://github.com/Azure/azure-sdk-for-net/issues/45620
autorest.csharp is fully working for greenfield MPG from TypeSpec but in order to get brownfield to work it will require a lot of code changes. Knowing we will be swapping to MGC we decided to only support brownfield from mpg which means we need to get the scaffolding for the azure emitter in place as soon as possible. This will allow us to being to parallelize the many tasks that will be necessary in order to get MPG brownfield generatable directly from TypeSpec.
This PR aims to only put the scaffolding in place such that we can start that work so I expect a lot of follow up issues to arise from this review. If you feel that your feedback can be addressed in a follow up item please indicate that and we will link the follow up issue in the comment resolution.
There are two parts of the plugin.
emitter: which doesn't contain much logic apart from specify the plugin name for now
generator: inherits ClientModelPlugin, no logic, will contain visitor and abstraction to manipulate the generation output
- Set up folder structure for azure-plugin as below
packages
|- npm
|- http-client-csharp
|- emitter
|- eng
|- pipelines
|- scripts
|- generator
|- Azure.Generator
- Set a test to ensure the basic generation
- Set up CI to ensure the build and verify basic generation
- How it works: The CSharp client generator is wrapped by the emitter and the emitter will be executed during the TypeSpec compilation as below:
npx tsp compile main.tsp --emit @azure-tools/csharp-plugin
It might be helpful to add some documentation to the PR to describe how this plugin will be used to generate clients in the repo.
I'm not a fan of the top-level azure-plugin directory. In the current hierarchy, seems that putting it under tools would make sense. Otherwise, maybe something like code-generation?
I like tools too. tools/generator/azure-plugin or tools/typespec/generator/AzureClientPlugin would seem like logical places to find something like this to me.
It might be helpful to add some documentation to the PR to describe how this plugin will be used to generate clients in the repo.
Did not demonstrate anything about how it works since I haven't really implemented any functionality in this PR yet. As the other comment, I can work on the basic functionality and add a script and test for it, add the description about the functionality.
In the meanwhile, please continue the discussion about the naming and the folder structure.
I'm not a fan of the top-level azure-plugin directory. In the current hierarchy, seems that putting it under tools would make sense. Otherwise, maybe something like code-generation?
I like tools too.
tools/generator/azure-pluginortools/typespec/generator/AzureClientPluginwould seem like logical places to find something like this to me.
how about tools/azure-plugin? did not put generator in between because we need to have generator and emitter folder within azure-plugin.
tools
|- azure-plugin
|- emitter
|- eng
|- pipelines
|- scripts
|- generator
|- Azure.Generator
I'm not a fan of the top-level
azure-plugindirectory. In the current hierarchy, seems that putting it undertoolswould make sense. Otherwise, maybe something likecode-generation?
@jsquire what if we called it http-client-csharp which I think will end up being the name of the emitter. We can also use a packages folder which is common in most js repos so it would look like
- packages/
- http-client-csharp/
- generator/
- emitter/
- http-client-csharp/
I'm not a fan of the top-level azure-plugin directory. In the current hierarchy, seems that putting it under tools would make sense. Otherwise, maybe something like code-generation?
I like tools too.
tools/generator/azure-pluginortools/typespec/generator/AzureClientPluginwould seem like logical places to find something like this to me.
Not sure I like tools since we will be publishing an npm package from the directory. I lean towards ./packages/http-client-csharp as mentioned in another comment.
Its also not a "tool" that you should invoke like the current items which are exe / scripts. This will be published and you should consume it from npm.
I'm not a fan of the top-level
azure-plugindirectory. In the current hierarchy, seems that putting it undertoolswould make sense. Otherwise, maybe something likecode-generation?@jsquire what if we called it
http-client-csharpwhich I think will end up being the name of the emitter. We can also use apackagesfolder which is common in most js repos so it would look like
packages/
http-client-csharp/
- generator/
- emitter/
Not a fan of http-client - unless you're embedded in the generator space, it isn't clear that this is a generator plugin. Given that HttpClient has its own connotation in .NET, that's going to be confusing. To me, packages would imply "these are the things we ship on NuGet."
I like the proposed tools/generator-plugin-azure personally. If we think there will be more generator-related stuff, I'm good with tools/generator/plugin-azure or generator/plugins/azure or something like that. My goal is to have something that strongly indicates "this is for the generator and its a plugin."
I'm not a fan of the top-level azure-plugin directory. In the current hierarchy, seems that putting it under tools would make sense. Otherwise, maybe something like code-generation?
I like tools too.
tools/generator/azure-pluginortools/typespec/generator/AzureClientPluginwould seem like logical places to find something like this to me.Not sure I like tools since we will be publishing an npm package from the directory. I lean towards
./packages/http-client-csharpas mentioned in another comment.
Personally, I'd make packages a child. We're a .NET repository so any raw "packages" reference is going to imply NuGet. I could be talked into a ./packages/npm/typespec-plugin-azure or something for the output, but that doesn't feel right for the source.
I'm not a fan of the top-level azure-plugin directory. In the current hierarchy, seems that putting it under tools would make sense. Otherwise, maybe something like code-generation?
I like tools too.
tools/generator/azure-pluginortools/typespec/generator/AzureClientPluginwould seem like logical places to find something like this to me.Not sure I like tools since we will be publishing an npm package from the directory. I lean towards
./packages/http-client-csharpas mentioned in another comment.Personally, I'd make
packagesa child. We're a .NET repository so any raw "packages" reference is going to imply NuGet. I could be talked into a./packages/npm/typespec-plugin-azureor something for the output, but that doesn't feel right for the source.
I'm not sure I like this being in the .NET repo but assuming I'm convinced and we do end up with this in the .NET repo I agree with Jesse here that we should not have a root level packages folder.
I'm not sure I like this being in the .NET repo but assuming I'm convinced and we do end up with this in the .NET repo I agree with Jesse here that we should not have a root level packages folder.
Talking offline with the .net team including @jsquire we agreed on ./packages/npm/http-client-csharp. Reasoning being that npm would help clarify its not a nuget package.
I was thinking about @weshaggard's comments, and had the following questions: Since AzureClientPlugin inherits from ClientModelPlugin (ClientPlugin? ThirdPartyClientPlugin?), changes to the base type could cause changes to the Azure plugin. This seems like it could introduce problems similar to the ones we have today where changes in one repo cause breaks in another repo, but we don't catch them until later and it's a lot of time and expense to mitigate.
I'd love if the story here could solve those problems to make our generation flows simpler and less costly going forward -- this will help the team scale better if we're not spending time on these issues as we do today.
An example of the kind of problem we could run into with the Azure client plugin inheriting from the 3p client plugin is -- say we wanted to change the 3p client plugin to write request URIs differently. Would Azure clients start writing request URIs differently, even though we weren't intending to change that pattern? If they did, at what point in the cycle of (a) update 3p client plugin in TypeSpec repo (b) update version of 3p client plugin in azure-sdk-for-net repo (c) regenerate Azure clients, would we catch this? If we didn't catch it for a few weeks, that could be a problem.
Then, say we could catch it at development time for making the change to the 3p client plugin. What if this is a very easy, simple change for that plugin and improves 3p client code a great deal. But, because it breaks how Azure clients are generated (due to plugin inheritance), the work to make the small, simple change requires also reworking the Azure client plugin to decouple the piece that writes request URIs. Now the cost of making a small change has increased a lot, and maybe it prevents us from improving 3p clients, or makes it really expensive for our team to make simple enhancements like this.
Is there an approach where we could isolate these kind of changes so that simple changes were low cost, and didn't add expense by having downstream repercussions?
An example of the kind of problem we could run into with the Azure client plugin inheriting from the 3p client plugin is -- say we wanted to change the 3p client plugin to write request URIs differently. Would Azure clients start writing request URIs differently, even though we weren't intending to change that pattern? If they did, at what point in the cycle of (a) update 3p client plugin in TypeSpec repo (b) update version of 3p client plugin in azure-sdk-for-net repo (c) regenerate Azure clients, would we catch this? If we didn't catch it for a few weeks, that could be a problem.
Basically, MGC provides the framework of SDK generation, our AzureClientPlugin is an extension plugin to provide the required features of Azure. As explained in this comment, we have a fixed version of MGC for AzureClientPlugin. We should be able to detect the unexpected changes for SDK client code during the version bump, we will need to test it thoroughly during the version bump and make sure the regression functionalities are working as expected.
Then, say we could catch it at development time for making the change to the 3p client plugin. What if this is a very easy, simple change for that plugin and improves 3p client code a great deal. But, because it breaks how Azure clients are generated (due to plugin inheritance), the work to make the small, simple change requires also reworking the Azure client plugin to decouple the piece that writes request URIs. Now the cost of making a small change has increased a lot, and maybe it prevents us from improving 3p clients, or makes it really expensive for our team to make simple enhancements like this.
Is there an approach where we could isolate these kind of changes so that simple changes were low cost, and didn't add expense by having downstream repercussions?
I will leave this to @m-nash, I think there are flexible ways to handle this, such as
- keep the basic plugin implementation unchanged and create another plugin of the base plugin for the usage requiring simple changes.
- override the default behavior in
AzureClientPluginlike we are going to implement for Azure
We should be able to detect the unexpected changes for SDK client code during the version bump, we will need to test it thoroughly during the version bump and make sure the regression functionalities are working as expected.
Yeah, I think this is the crux of my question -- are we happy with this process, or is it worth seeing if we can find a flow that would cost our team less time going forward.
I'd love to look at this through the lens of the specific example I mentioned. Say, for example, we make some changes to the SCM generator to change how URIs are written to requests in SCM-based clients. We update the SCM generator, test 3p clients, merge, mark the GH issue as resolved, and consider it done. Depending on how often the version bumps happen, maybe we discover a few days/a week later that this update breaks the Azure client generator patterns. What happens now? Who would own 1) redesigning the SCM plugin/3p client pattern and reworking the GH issue that had been closed? Or 2) updating the Azure plugin to accommodate the new SCM plugin version and generate Azure clients correctly? Did we account for that work in our costing of the feature? Is it useful work, or is it work we could have avoided if we had decided on a different architecture than the Azure plugin inheriting from the SCM plugin? Does it add to the cost of developing .NET generator features? How could we set ourselves up today to minimize those costs to help our team scale?
I'm wondering if there's value in considering an alternate architecture where there is a base "ClientPlugin" that both SCM and Azure client plugins could inherit from. This could enforce basic patterns like constructors, convenience methods, protocol methods, and request creation helpers, but then Azure clients and SCM clients could vary the implementation of these things independently without one causing unforeseen development costs to the other. Have we considered an architecture like this for plugins? If so, I'd love to learn what the thinking was that led us to decide on the current architecture over an architecture like that, if you're willing to share the motivation around that.
We should be able to detect the unexpected changes for SDK client code during the version bump, we will need to test it thoroughly during the version bump and make sure the regression functionalities are working as expected.
Yeah, I think this is the crux of my question -- are we happy with this process, or is it worth seeing if we can find a flow that would cost our team less time going forward.
I'd love to look at this through the lens of the specific example I mentioned. Say, for example, we make some changes to the SCM generator to change how URIs are written to requests in SCM-based clients. We update the SCM generator, test 3p clients, merge, mark the GH issue as resolved, and consider it done. Depending on how often the version bumps happen, maybe we discover a few days/a week later that this update breaks the Azure client generator patterns. What happens now? Who would own 1) redesigning the SCM plugin/3p client pattern and reworking the GH issue that had been closed? Or 2) updating the Azure plugin to accommodate the new SCM plugin version and generate Azure clients correctly? Did we account for that work in our costing of the feature? Is it useful work, or is it work we could have avoided if we had decided on a different architecture than the Azure plugin inheriting from the SCM plugin? Does it add to the cost of developing .NET generator features? How could we set ourselves up today to minimize those costs to help our team scale?
I'm wondering if there's value in considering an alternate architecture where there is a base "ClientPlugin" that both SCM and Azure client plugins could inherit from. This could enforce basic patterns like constructors, convenience methods, protocol methods, and request creation helpers, but then Azure clients and SCM clients could vary the implementation of these things independently without one causing unforeseen development costs to the other. Have we considered an architecture like this for plugins? If so, I'd love to learn what the thinking was that led us to decide on the current architecture over an architecture like that, if you're willing to share the motivation around that.
@m-nash Could you share the thoughts and considerations about the current inheritance design from SCM ClientModelPlugin, not the base CodeModelPlugin?
I talked to @m-nash today, and learned a lot more about this. Here are my takeaways:
- For my "assign a URI on a request" example, "abstractions" like building a URI should be general enough that the Azure plugin could override the abstraction via inheritance and it should work without a lot of additional work on the Azure side. Where type names have changed across SCM/Azure.Core, the Azure plugin will likely already have inherited from the abstraction, so changes to Azure client patterns should already be insulated from changes to SCM client patterns.
- If we want to change larger patterns (one example might be moving where the URI is created to a different client method), the Azure plugin could override some method providers and write different method implementations that way.
- We can change how internal client code is written later on, even if third-party types inherit from SCM plugin types, because we don't plan to GA that aspect of the generator types, but there is always a tradeoff there based on the risk of breaking changes to deriving plugins. There are things like feature flags and version pinning to mitigate those issues.
- If we wanted to remove the cost of making changes to the Azure plugin when implementing SCM-based client features at some point in the future, we could always derive a new plugin from ClientModelPlugin in the future if needed. i.e. something like:
Given all of that, my immediate concerns about plugin inheritance are satisfied, and the cross-repo stuff we just have to live with. Thanks for the discussion on all of this!
@weshaggard Folder structure updated as agreed, please take another look.
API change check
API changes are not detected in this pull request.