Enhance add databaseDepMap by maven dependency#4886
Merged
rujche merged 5 commits intoMar 6, 2025
Conversation
rujche
requested review from
JeffreyCA,
hemarina,
vhvb1989,
wbreza and
weikanglim
as code owners
Contributor
There was a problem hiding this comment.
PR Overview
This PR enhances database dependency detection for Maven projects by including additional Azure-specific dependencies and fixes missing support for MySQL in the detection and recommendation flows.
- Updated Maven dependency detection to include Azure Spring starters for MySQL and PostgreSQL, as well as adding detection for Redis and Mongo.
- Added logic for recommending Azure Database for MySQL flexible server and mapping MySQL to a resource type in add_configure.go.
- Modified infrastructure confirmation logic to prompt for database names and assign configuration for MySQL, though with a potential issue with field assignment.
Reviewed Changes
| File | Description |
|---|---|
| cli/azd/internal/appdetect/java.go | Expanded dependency matching to include Azure Spring starters for MySQL, PostgreSQL, Redis, and Mongo. |
| cli/azd/internal/repository/detect_confirm.go | Added recommendation for Azure Database for MySQL flexible server. |
| cli/azd/internal/repository/infra_confirm.go | Adjusted database name prompting and configuration assignment for MySQL along with PostgreSQL. |
| cli/azd/internal/cmd/add/add_configure.go | Extended DbMap to map MySQL dependencies to a new resource type. |
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
PR Overview
This PR enhances the detection of Maven dependencies by adding support for Azure-specific MySQL and PostgreSQL dependencies, as well as adding detection for Redis and MongoDB dependencies. It also updates related repository confirmation logic and configuration mapping accordingly.
- Enhances dependency detection in the Java project scanner to include additional Azure Maven dependencies.
- Updates repository confirmation logic to handle MySQL and adjusts mapping in configuration.
- Updates spell-check configuration to include the term "springframework".
Reviewed Changes
| File | Description |
|---|---|
| cli/azd/internal/appdetect/java.go | Adds multiple Maven dependency checks for MySQL, PostgreSQL, Redis, and MongoDB; leaves a todo for Cosmos DB. |
| cli/azd/internal/repository/detect_confirm.go | Incorporates a new case for MySQL to recommend the appropriate Azure service. |
| cli/azd/.vscode/cspell.yaml | Updates the spell-check word list. |
| cli/azd/internal/repository/infra_confirm.go | Adjusts switch cases to validate and assign configuration for MySQL along with PostgreSQL and Redis. |
| cli/azd/internal/cmd/add/add_configure.go | Updates the resource type mapping to support MySQL. |
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
saragluna
reviewed
Mar 5, 2025
weikanglim
reviewed
Mar 5, 2025
weikanglim
reviewed
Mar 5, 2025
Contributor
weikanglim
left a comment
weikanglim
left a comment
There was a problem hiding this comment.
@rujche This looks good to me. I recommend removing the infra_confirm.go change as it's the legacy codepath with compose not enabled. Otherwise, I'm ready to merge this on the next iteration.
Member
Author
|
Thanks for the quick response!
Done. Please help to review again. |
Contributor
There was a problem hiding this comment.
PR Overview
This PR enhances the database dependency mapping by adding support for additional Maven dependencies related to MySQL, PostgreSQL, Redis, and MongoDB while noting a future addition for Cosmos. Key changes include:
- Enhancing dependency detection in java.go to check both native and Azure Spring dependency variants.
- Updating UI recommendations in detect_confirm.go by adding MySQL and mapping database types in add_configure.go.
- Updating the spelling configuration in cspell.yaml.
Reviewed Changes
| File | Description |
|---|---|
| cli/azd/internal/appdetect/java.go | Extended Maven dependency detection for MySQL, PostgreSQL, Redis, and MongoDB. |
| cli/azd/internal/repository/detect_confirm.go | Added MySQL to the recommended services list. |
| cli/azd/.vscode/cspell.yaml | Updated spelling configuration with an entry for "springframework". |
| cli/azd/internal/cmd/add/add_configure.go | Added MySQL mapping in the database configuration map. |
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
28 tasks
This file contains hidden or 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
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.
appdetect.DbMySql.DbCosmosis not added now, my current plan is add it in next PR, after current PR merged.