Support azd hooks out of azure.yaml, Part I of merging azd operation to hooks #4244
Conversation
…efine hooks outside of azure.yaml as individual files with the name azd.hooks.[yaml | yml]. The file can be placed in the /infra folder for project level hooks or inside service's folder for service level hooks. This will allow generators like Aspire to include hooks as part of the infrastructure code (w/o touching azure.yaml). Then, when running azd infra synth, the hooks are part of the infrastructure and users can delete the infra folder to go back to full Aspire in-memory generation w/o touching azure.yaml. After this PR, I am planning to move the azd operations to become a valid built-in hook and delete the alpha feature for azd operations and the entire feature in favor of just using the hooks strategy
vhvb1989
requested review from
ellismg,
wbreza,
hemarina and
weikanglim
as code owners
| @@ -130,6 +130,24 @@ func Load(ctx context.Context, projectFilePath string) (*ProjectConfig, error) { | |||
| return nil, fmt.Errorf("parsing project file: %w", err) | |||
| } | |||
|
|
|||
| projectConfig.Path = filepath.Dir(projectFilePath) | |||
There was a problem hiding this comment.
I wouldn't put the hooks in /infra folder to make the not service specific.
If you put in the infra folder then that means they are related to infra, which they can be, but that isn't always true.
I recommend either:
- Put them in the root
- Create a new folder, something like hooks, or scripts/hooks.
There was a problem hiding this comment.
100% agree here. The hooks config file should either be in the same folder as the azure.yaml for project level command hooks or in a project folder of a service for service specific hooks.
Only provision hooks are related to infra and that shouldn't override configuration for all hook configuration.
There was a problem hiding this comment.
Adding it to /infra is the fundamental intention of this PR, :)
azd.operations are infrastructure-extensions.
When translating to azd.operations to hooks, we would expect the azd.hooks in the /infra folder to use either postprovision or preprovision. Otherwise, hooks would be defined in azure.yaml.
I don't expect folks to migrate from using azure.yaml to use azd.hooks file in /infra folder. The intention is to allow projects like Aspire to generate the infrastructure including hooks (currently known as azd.operations).
Would you feel better if azd fails when using azd.hooks in /infra to define a non postprovision or preprovision hook?
However, I wouldn't find too much value in such validation.
The expectation, when using Aspire to generate infrastructure (with azd infra synth) is that folks can easily delete the infra (by deleing the infra folder) to go back to auto-in-memory-generation. If the azd.hooks file goes out of /infra, we would be adding an extra step for cleaning the infra files.
There was a problem hiding this comment.
How about updating this to work with any file in the project that is prefixed with azd.hooks, but not make /infra the standard. As-in, if the user Really wants to put it in infra. We don't stop them, but if they want to put it in the route, they can. They can put it wherever they want in your particular case. You want it in infra and that should be fine with a flexible design, But I don't think we should force them to put it in infra folder.
There was a problem hiding this comment.
Can we use this PR as the first step on that direction? Then wait and measure usage and if customers ask for more?
My expectation is that customers will continue to use azure.yaml for hooks, as you have the yaml schema to help you use IntelliSense and spot errors. The azd.hooks file is coming right now as an alternative for code-generators.
There was a problem hiding this comment.
The azd operations is an alpha feature and is subject to change. As we discussed these really are the same as hooks with more hard coded operations vs generic scripts. Because of this I would not want to see azd operations continue forward as its own unique feature for only infra related activities. For aspire scenarios it really shouldn't matter whether you generate this file in the infra folder or in the project root.
There was a problem hiding this comment.
For aspire scenarios it really shouldn't matter whether you generate this file in the infra folder or in the project root.
It matters a lot!! For customers running azd infra synth, the output goes to the /infra folder.
We used to output to /infra folder + services' folders and got user's feedback/request to consolidate it all in one single folder (so they could remove it if they want to go back to Aspire-generation).
That's why it is fundamental in this case to start by supporting the azd.hooks file at /infra folder, to solve the main user-case we have right now.
In the future, we can expand to all folders if we want (or customers ask for it). Right now, I think it would be much more than what we need.
And yes, after this, azd operations alpha feature would be retired :)
There was a problem hiding this comment.
I understand that the infra gets generated in the infra folder but this doesn't mean and hooks configuration should be here also. I'd strongly advise we don't include the hooks yaml in the infra and move this to the root as suggested above.
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
As part of merging azd operation and hooks, this PR allows users to define hooks outside of azure.yaml as individual files with the name azd.hooks.[yaml | yml].
The file can be placed in the /infra folder for project level hooks or inside service's folder for service level hooks.
This will allow generators like Aspire to include hooks as part of the infrastructure code (w/o touching azure.yaml). Then, when running azd infra synth, the hooks are part of the infrastructure and users can delete the infra folder to go back to full Aspire in-memory generation w/o touching azure.yaml.
After this PR, I am planning to move the azd operations to become a valid built-in hook and delete the alpha feature for azd operations and the entire feature in favor of just using the hooks strategy
fix: #4394