-
Notifications
You must be signed in to change notification settings - Fork 228
extension: support existing
resource for ai build start
#5193
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
base: main
Are you sure you want to change the base?
Conversation
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
edisplay | ||
eignore | ||
eloading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are this used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to add all of these new non-words?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added due to spell check error: https://github.com/Azure/azure-dev/actions/runs/15026950586/job/42229968592
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good - added a few comments I'd like to see tested/addressed.
edisplay | ||
eignore | ||
eloading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to add all of these new non-words?
@@ -69,6 +70,7 @@ message ComposedResource { | |||
string type = 2; | |||
bytes config = 3; | |||
repeated string uses = 4; | |||
string resource_id = 5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the ComposedResource
already contains a ResourceId
, do we really need to modify the AddResourceRequest
?
func NewComposeService( | ||
lazyAzdContext *lazy.Lazy[*azdcontext.AzdContext], | ||
env *environment.Environment, | ||
envManager environment.Manager, | ||
) azdext.ComposeServiceServer { | ||
return &composeService{ | ||
lazyAzdContext: lazyAzdContext, | ||
env: env, | ||
envManager: envManager, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to be careful adding non-lazy dependencies to the gRPC server side services because the component initialization can fail. For example, try running the command in a fold that doesn't have an azd project or environment yet and this will likely fail.
Take a look at some of the other gRPC services for examples to use lazy components.
fix #5103

