-
Notifications
You must be signed in to change notification settings - Fork 176
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
keep port when exists #4018
keep port when exists #4018
Conversation
cli/azd/pkg/apphost/testdata/TestAspireDockerGeneration-nodeapp.snap
Outdated
Show resolved
Hide resolved
Something about using the negatives here to control which of the two plans we are on seems really weird to me. Is there another design when we either have two fields or use nil or something? I dunno - this feels a little hacky to me and I worry about us missing a |
FWIW - I am somewhat susceptible to @vhvb1989's argument (made offline, at standup) that while this is pretty hacky, we expect that most of this code will go away in the next 3 months as we move this generation logic out of azd and into aspire and so long term we may not be stuck with this negative port number hack. If doing it a less hacky way ends up feeling like it adds a bunch more code and overall is a worse tactical move, given the impending halflife of all this code, I think I'm okay with what we have. It might be nice, however, to fix things up such that we don't have a negative port number in the generated IaC. I think someone looking at:
Will have a pretty good understanding of what this does. Someone looking at:
Will almost certainly think something is wrong, since having a negative number there looks very confusing. Not sure how you feel about this, @weikanglim, seemed like you were nodding along with the reasoning for the hack during standup. I think if the weirdness is not visible outside the |
@ellismg I'm in the same place of avoiding slightly awkward observable UX if possible. The other thing I think about is that it's also hard to predict if code we write today is truly going away, and the observable contract we would have to honor makes it harder for us to make changes again. That being said, I could see this as a tactical win if the alternative is near impossible (for example, the solution is incomplete due to some missing info from the manifest). I started a thread here just to understand more. |
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
fix: #4005
Using negative target port when resolving the
target port
as the way to communicate that the port should not be changed. When azd builds a net project and gets a port number from the build result, the port won't be used if the target port in the yaml template is negative. Instead, the negative target port is used (making it positive)This fix is required for Aspire 8.1. For 8.0, folks need to add
launchProfileName: null
when adding a project so they can use this fix