Skip to content
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

feat(autoconfig): autoconfig module #1308

Closed
wants to merge 20 commits into from
Closed

Conversation

diegomarquezp
Copy link
Contributor

has shared properties to be used by generated spring autoconfigured client beans

has shared properties to be used by generated spring autoconfigured
client beans
@diegomarquezp diegomarquezp marked this pull request as ready for review November 25, 2022 16:04
@zhumin8
Copy link
Contributor

zhumin8 commented Dec 1, 2022

Perhaps get some test coverage with a simple test with fake credential key?
For reference: https://www.baeldung.com/spring-boot-testing-configurationproperties#binding-POJOs

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 2, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these global properties specific to the auto-generated modules? Shouldn't they be in autoconfiguration core?

Copy link
Contributor

@elefeint elefeint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as far as I'm concerned, but I've added Mike's question to the design doc's section.

@diegomarquezp
Copy link
Contributor Author

Closing as not planned

zhumin8 added a commit to googleapis/sdk-platform-java that referenced this pull request Dec 16, 2022
…1123)

As discussed off-line, instead of depending on a new `global-properties` module, use `CredentialsProvider` bean from `spring-cloud-gcp-autoconfigure` instead.
- Replace dependency from `core` to `autoconfigure` module.
- Revert #1071 and implement changes shown in [poc](zhumin8/language_poc1#7)
- Minor code cleanups and util method extracted from `SpringPropertiesClassComposer.java`. 

Note: #1071 and GoogleCloudPlatform/spring-cloud-gcp#1308 will be obsolete after this change.
Also, conflicts needs to be resolved between this pr and #1110 depending on merging order.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants