Skip to content

[IAM] Remove old quickstart, add dependencies #3752

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Sep 17, 2020
Merged

[IAM] Remove old quickstart, add dependencies #3752

merged 7 commits into from
Sep 17, 2020

Conversation

melaniedejong
Copy link
Contributor

Removes old Quickstart. Also adds dependencies for IAM Credentials and Troubleshooter, which will be used to keep the client library install instructions up-to-date.

  • I have followed Sample Format Guide

  • pom.xml parent set to latest shared-configuration

  • Appropriate changes to README are included in PR

  • Tests pass: mvn clean verify required

  • Lint passes: mvn -P lint checkstyle:check required

  • Please merge this PR for me once it is approved.

    • Please coordinate with me though because I need to publish a documentation CL simultaneously (cl/332295147)

@melaniedejong melaniedejong requested a review from a team September 17, 2020 20:28
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 17, 2020
lesv
lesv reviewed Sep 17, 2020
View reviewed changes
iam/api-client/src/main/java/iam/snippets/Quickstart.java
@@ -1,4 +1,4 @@
/* Copyright 2018 Google LLC
/* Copyright 2020 Google LLC
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to update this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is technically a new quickstart, I'm just renaming it to avoid confusion. (See #2829)

lesv
lesv reviewed Sep 17, 2020
View reviewed changes
lesv
lesv reviewed Sep 17, 2020
View reviewed changes
iam/api-client/src/main/java/iam/snippets/Quickstart.java Outdated
GoogleNetHttpTransport.newTrustedTransport(),
JacksonFactory.getDefaultInstance(),
new HttpCredentialsAdapter(credential))
.setApplicationName("quickstart")
.setApplicationName("service-accounts")
Copy link
Contributor

@lesv lesv Sep 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It might be better if you used "Java IAM Quickstart snippet"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

lesv
lesv suggested changes Sep 17, 2020
View reviewed changes
Copy link
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

PTAL

@lesv
Copy link
Contributor

lesv commented Sep 17, 2020

Java 11 failed.

@lesv
Copy link
Contributor

lesv commented Sep 17, 2020

Java 11

------------------------------------------------------------
- testing iam/api-client
------------------------------------------------------------
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 2.034 s <<< FAILURE! - in iam.snippets.QuickstartTests
[ERROR] iam.snippets.QuickstartTests.testQuickstart  Time elapsed: 2.03 s  <<< ERROR!
java.lang.NullPointerException
	at iam.snippets.Quickstart.setPolicy(Quickstart.java:170)
	at iam.snippets.Quickstart.addBinding(Quickstart.java:122)
	at iam.snippets.QuickstartTests.testQuickstart(QuickstartTests.java:126)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:377)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:138)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:465)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:451)

[ERROR] Errors:
[ERROR]   QuickstartTests.testQuickstart:126 » NullPointer
[ERROR] Tests run: 16, Failures: 0, Errors: 1, Skipped: 0

@lesv
Copy link
Contributor

lesv commented Sep 17, 2020

The Java 8 and Java 11 typically run at the same time.

@lesv
Copy link
Contributor

lesv commented Sep 17, 2020

The problem is in L129 in the tests - you probably should add an UUID in there somewhere. (and make sure to remove it)

@lesv
Copy link
Contributor

lesv commented Sep 17, 2020

That said, things are passing now. I suspect the test will be flakey, so consider my comment about a UUID.

@melaniedejong
Copy link
Contributor Author

What would I add a UUID to? The service account already has one, and the project ID is from an environment variable.

@lesv lesv merged commit ba096de into GoogleCloudPlatform:master Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lesv lesv lesv approved these changes

Assignees

@stephaniewang526 stephaniewang526

Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@melaniedejong @lesv @stephaniewang526