New feature: Update spring boot properties file for postgresql #4707
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rujche
requested review from
ellismg,
wbreza,
vhvb1989,
hemarina,
weikanglim and
JeffreyCA
as code owners
rujche
requested review from
Copilot,
saragluna and
weikanglim
and removed request for
weikanglim,
wbreza,
JeffreyCA,
ellismg,
vhvb1989,
hemarina and
Copilot
|
Hi, @saragluna , @weikanglim |
weikanglim
reviewed
Jan 17, 2025
| @@ -217,7 +217,7 @@ var {{bicepName .Name}}Env = map(filter({{bicepName .Name}}AppSettingsArray, i = | |||
| module {{bicepName .Name}} 'br/public:avm/res/app/container-app:0.8.0' = { | |||
| name: '{{bicepName .Name}}' | |||
| params: { | |||
| name: '{{.Name}}' | |||
| name: '{{containerAppName .Name}}' | |||
There was a problem hiding this comment.
Curious about the change here -- we did intentionally move away from containerAppName .Name to .Name in #4434
There was a problem hiding this comment.
we did intentionally move away from containerAppName .Name to .Name in #4434
-
Do you mean change from
.NametocontainerAppName .Namein easy-init: allow for longer container app names #4434 ? -
In think the mistake (missing
containerAppName) is added in this PR: https://github.com/Azure/azure-dev/pull/4455/files
There was a problem hiding this comment.
I see. I think when #4455 was made, we also started adding upfront name validation in azd add (UI layer, not at the schema layer).
Keeping it as .Name instead of containerAppName .Name (which does the truncation) transformation may help with #4653, so I think .Name still looks better directionally. What do you think?
There was a problem hiding this comment.
How about doing like this:
- In current PR, keep using
containerAppName .Namebecause it can solve the problem appeared in the sample project for current PR: https://github.com/azure-javaee/azure-spring-boot-samples/blob/788cc9a14f3fa2bfa826b0e875ba6dd729726dfa/postgresql/spring-cloud-azure-starter-jdbc-postgresql/spring-cloud-azure-postgresql-sample/pom.xml#L39 - For [Issue] Failed to deploy app when app name too long or different app name has same long prefix #4653, we can fix it in another PR. In that PR, the same name prefix should be considered,
Improve func: appendToCommaSeperatedValues
saragluna
reviewed
Jan 22, 2025
| } | ||
| type propertyMergeFunc func(string, string) string | ||
|
|
||
| const azureProfileName = "azure" |
There was a problem hiding this comment.
what if in the user's application already have one azure profile?
There was a problem hiding this comment.
Good point.
In this case, original value of spring.profiles.active will not be changed. See these test cases:
And here is related source code:
Comment on lines
21
to
24
| const placeholderPostgresHost = "${POSTGRES_HOST}" | ||
| const placeholderPostgresPort = "${POSTGRES_PORT}" | ||
| const placeholderPostgresDatabase = "${POSTGRES_DATABASE}" | ||
| const placeholderPostgresUsername = "${POSTGRES_USERNAME}" |
There was a problem hiding this comment.
These are the conventional env names that defined in azd.
There was a problem hiding this comment.
I mean these should be defined in a common place, instead of being defined in java
There was a problem hiding this comment.
Good point. Updated in this commit: cdc9acd
…um of 125 characters. (lll)
// G101 : Potential hardcoded credentials (gosec)
|
Hi, @saragluna , @weikanglim |
…o to app_init.go.
…ng-boot-properties-file-for-postgresql
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
|
Hi, @saragluna , @weikanglim . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1. Description
New feature: Update spring boot properties file for postgresql.
2. Try this feature
Link to sample project: https://github.com/azure-javaee/azure-spring-boot-samples/blob/788cc9a14f3fa2bfa826b0e875ba6dd729726dfa/postgresql/spring-cloud-azure-starter-jdbc-postgresql/spring-cloud-azure-postgresql-sample/pom.xml#L39
Screenshot before running
azd init:azd init:todos in current PR, my current plan is do them after current PR merged.