-
Notifications
You must be signed in to change notification settings - Fork 693
Creates spring-cloud-gcp-autoconfigure module #276
Changes from 9 commits
f2624c8
932a437
1ec45bf
41aea29
9213f28
c669ea9
203d0cf
0a4362a
2942c60
13dd3d6
8303715
0f609e0
2287602
77364d4
b231149
ac3dedb
2de9f93
a53aef5
902eea3
2b7852a
5813ae8
0d47060
c5486c7
2ccc845
1da213a
96c58fd
2d39c0d
7c3da59
252a533
13c45aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<project xmlns="http://maven.apache.org/POM/4.0.0" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
<parent> | ||
<artifactId>spring-cloud-gcp</artifactId> | ||
<groupId>org.springframework.cloud</groupId> | ||
<version>1.0.0.BUILD-SNAPSHOT</version> | ||
</parent> | ||
<modelVersion>4.0.0</modelVersion> | ||
|
||
<artifactId>spring-cloud-gcp-autoconfigure</artifactId> | ||
<name>Spring Cloud GCP Autoconfigure Module</name> | ||
|
||
<dependencies> | ||
<dependency> | ||
<groupId>org.springframework.boot</groupId> | ||
<artifactId>spring-boot-autoconfigure</artifactId> | ||
</dependency> | ||
<!-- Configuration processor --> | ||
<dependency> | ||
<groupId>org.springframework.boot</groupId> | ||
<artifactId>spring-boot-configuration-processor</artifactId> | ||
<optional>true</optional> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.springframework.cloud</groupId> | ||
<artifactId>spring-cloud-gcp-core</artifactId> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this dependency needed currently? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we need it to import There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider the case someone decides to add the auto-configuration of GCP to their stack but there are scenarios where they might not use it yet. IMO, this should be |
||
</dependency> | ||
<dependency> | ||
<groupId>org.springframework.cloud</groupId> | ||
<artifactId>spring-cloud-gcp-config</artifactId> | ||
<optional>true</optional> | ||
</dependency> | ||
</dependencies> | ||
</project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
org.springframework.boot.autoconfigure.EnableAutoConfiguration=\ | ||
org.springframework.cloud.gcp.autoconfigure.core.GcpContextAutoConfiguration,\ | ||
org.springframework.cloud.gcp.autoconfigure.config.GcpConfigAutoConfiguration | ||
|
||
org.springframework.cloud.bootstrap.BootstrapConfiguration=\ | ||
org.springframework.cloud.gcp.autoconfigure.config.GcpConfigAutoConfiguration,\ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is correct, because it's for |
||
org.springframework.cloud.gcp.autoconfigure.core.GcpContextAutoConfiguration | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know enough about that stuff to be positive but the fact those classes are both auto-config and bootstrap config is a smell IMO. (nothing to do with this issue per se but I thought I'd mention it). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<project xmlns="http://maven.apache.org/POM/4.0.0" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
<parent> | ||
<artifactId>spring-cloud-gcp</artifactId> | ||
<groupId>org.springframework.cloud</groupId> | ||
<version>1.0.0.BUILD-SNAPSHOT</version> | ||
</parent> | ||
<modelVersion>4.0.0</modelVersion> | ||
|
||
<artifactId>spring-cloud-gcp-config</artifactId> | ||
<name>Spring Cloud GCP Config Module</name> | ||
|
||
<dependencies> | ||
<dependency> | ||
<groupId>org.springframework.cloud</groupId> | ||
<artifactId>spring-cloud-context</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.springframework.cloud</groupId> | ||
<artifactId>spring-cloud-gcp-core</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.springframework</groupId> | ||
<artifactId>spring-web</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>commons-logging</groupId> | ||
<artifactId>commons-logging</artifactId> | ||
</dependency> | ||
</dependencies> | ||
</project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,5 +22,9 @@ | |
<groupId>org.springframework</groupId> | ||
<artifactId>spring-core</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.springframework.boot</groupId> | ||
<artifactId>spring-boot-autoconfigure</artifactId> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cyclic dependency? Thanks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh! My bad. I see what is that. Not sure how have you arranged it with Stephane, but I feel like something in architecture is still missing. Maybe there is still something in the target modules what is Boot-based? So, that should be moved to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other spring-boot projects seem to be keeping their I reviewed our project and all For the config one, we might be able to make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Are you going to do those changes to avoid this Thanks |
||
</dependency> | ||
</dependencies> | ||
</project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,2 @@ | ||
org.springframework.cloud.bootstrap.BootstrapConfiguration=\ | ||
org.springframework.cloud.gcp.config.autoconfig.GcpConfigAutoConfiguration,\ | ||
org.springframework.cloud.gcp.core.autoconfig.GcpContextAutoConfiguration,\ | ||
org.springframework.cloud.gcp.config.GoogleConfigPropertySourceLocator | ||
org.springframework.cloud.gcp.config.GoogleConfigPropertySourceLocator | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New line in the end of file. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
provides: spring-cloud-gcp-autoconfigure, spring-cloud-gcp-config |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
provides: spring-cloud-gcp-autoconfigure, spring-cloud-gcp-core |
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.
This exclusion for
android-json
should be added because it indeed conflicts withcom.google.cloud.ServiceOptions.getServiceAccountProjectId(ServiceOptions.java:442)
use of theorg.json.JSONTokener.<init>(Ljava/io/InputStream;)V
constructor, which doesn't exist in the stripped downandroid-json
library.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.
Then should we exclude
android-json
from the other sources and use the one fromspring-boot-configuration-processor
?It's hard to fix this issue because I'm not seeing it, so I'm not sure to what extent this is happening because of your (or mine) local configuration.
In the meantime, my toy project is again picking up project ID and creds and I didn't do any change, not sure what happened but things work now...
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.
Maven loads dependencies in the order that they are declared. Since
spring-boot-configuration-processor
comes beforespring-cloud-gcp-core
in this POM,JSONTokenizer
fromandroid-json
should get loaded. I'm not sure why you're not seeing it.For me, actually just switching the order of
spring-boot-configuration-processor
andspring-cloud-gcp-core
fixes the build.Can you try running
mvn dependency:tree
on this POM to see in what order dependencies are loaded.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.
I think at this point we need to configure the annotation processor in the compiler plugin. You may lose AP support in the IDE but it's better than the current arrangement IMO.
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.
Only
spring-boot-configuration-processor
is importingandroid-json
for me.[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ spring-cloud-gcp-autoconfigure ---
[INFO] org.springframework.cloud:spring-cloud-gcp-autoconfigure:jar:1.0.0.BUILD-SNAPSHOT
[INFO] +- org.springframework.boot:spring-boot-autoconfigure:jar:1.5.7.RELEASE:compile
[INFO] | - org.springframework.boot:spring-boot:jar:1.5.7.RELEASE:compile
[INFO] +- org.springframework.boot:spring-boot-configuration-processor:jar:1.5.7.RELEASE:compile
[INFO] | - com.vaadin.external.google:android-json:jar:0.0.20131108.vaadin1:compile
[INFO] +- org.springframework.cloud:spring-cloud-gcp-core:jar:1.0.0.BUILD-SNAPSHOT:compile
[INFO] | +- com.google.cloud:google-cloud-core:jar:1.14.0:compile
[INFO] | | +- com.google.guava:guava:jar:20.0:compile
[INFO] | | +- joda-time:joda-time:jar:2.9.2:compile
[INFO] | | +- org.json:json:jar:20160810:compile
[INFO] | | +- com.google.http-client:google-http-client:jar:1.23.0:compile
[INFO] | | | - org.apache.httpcomponents:httpclient:jar:4.5.3:compile
[INFO] | | | +- org.apache.httpcomponents:httpcore:jar:4.4.6:compile
[INFO] | | | - commons-codec:commons-codec:jar:1.10:compile
[INFO] | | +- com.google.code.findbugs:jsr305:jar:3.0.0:compile
[INFO] | | +- com.google.api:api-common:jar:1.2.0:compile
[INFO] | | +- com.google.api:gax:jar:1.15.0:compile
[INFO] | | | +- com.google.auto.value:auto-value:jar:1.2:compile
[INFO] | | | +- org.threeten:threetenbp:jar:1.3.3:compile
[INFO] | | | - com.google.auth:google-auth-library-oauth2-http:jar:0.7.1:compile
[INFO] | | | +- com.google.auth:google-auth-library-credentials:jar:0.9.0:compile
[INFO] | | | - com.google.http-client:google-http-client-jackson2:jar:1.23.0:compile
[INFO] | | | - com.fasterxml.jackson.core:jackson-core:jar:2.8.10:compile
[INFO] | | +- com.google.protobuf:protobuf-java-util:jar:3.4.0:compile
[INFO] | | | +- com.google.protobuf:protobuf-java:jar:3.4.0:compile
[INFO] | | | - com.google.code.gson:gson:jar:2.8.1:compile
[INFO] | | +- com.google.api.grpc:proto-google-common-protos:jar:1.0.2:compile
[INFO] | | - com.google.api.grpc:proto-google-iam-v1:jar:0.1.26:compile
[INFO] | - org.springframework:spring-core:jar:4.3.11.RELEASE:compile
[INFO] +- org.springframework:spring-web:jar:4.3.11.RELEASE:compile
[INFO] | +- org.springframework:spring-aop:jar:4.3.11.RELEASE:compile
[INFO] | +- org.springframework:spring-beans:jar:4.3.11.RELEASE:compile
[INFO] | - org.springframework:spring-context:jar:4.3.11.RELEASE:compile
[INFO] | - org.springframework:spring-expression:jar:4.3.11.RELEASE:compile
[INFO] +- commons-logging:commons-logging:jar:1.2:compile
[INFO] +- org.springframework.cloud:spring-cloud-context:jar:1.2.3.RELEASE:compile
[INFO] | - org.springframework.security:spring-security-crypto:jar:4.2.3.RELEASE:compile
[INFO] +- junit:junit:jar:4.12:test
[INFO] | - org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] +- org.slf4j:slf4j-api:jar:1.7.25:compile
[INFO] +- org.mockito:mockito-core:jar:1.10.19:test
[INFO] | - org.objenesis:objenesis:jar:2.1:test
[INFO] - org.springframework.boot:spring-boot-starter-test:jar:1.5.7.RELEASE:test
[INFO] +- org.springframework.boot:spring-boot-test:jar:1.5.7.RELEASE:test
[INFO] +- org.springframework.boot:spring-boot-test-autoconfigure:jar:1.5.7.RELEASE:test
[INFO] +- com.jayway.jsonpath:json-path:jar:2.2.0:test
[INFO] | - net.minidev:json-smart:jar:2.2.1:test
[INFO] | - net.minidev:accessors-smart:jar:1.1:test
[INFO] | - org.ow2.asm:asm:jar:5.0.3:test
[INFO] +- org.assertj:assertj-core:jar:2.6.0:test
[INFO] +- org.hamcrest:hamcrest-library:jar:1.3:test
[INFO] +- org.skyscreamer:jsonassert:jar:1.4.0:test
[INFO] - org.springframework:spring-test:jar:4.3.11.RELEASE:test
[INFO] ------------------------------------------------------------------------
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.
@joaoandremartins Can you just switch the order of
spring-boot-configuration-processor
andspring-cloud-gcp-core
to fix the build for me?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.
I don't think you can exclude the android dependency and expect the APT to work. I believe it's no longer a dependency in Spring Boot 1.5.10 though, so upgrading to snapshots will fix it.