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

fix: move autovalue to annotation processor path #179

Merged
merged 15 commits into from Jan 28, 2021
Merged

fix: move autovalue to annotation processor path #179

merged 15 commits into from Jan 28, 2021

Conversation

elharo
Copy link
Contributor

@elharo elharo commented Jul 27, 2020

@elharo elharo requested a review from suraj-qlogic July 27, 2020 15:14
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 27, 2020
@elharo
Copy link
Contributor Author

elharo commented Jul 27, 2020

2020-07-27T15:15:25.6293721Z [ERROR] com.google.cloud.storage.contrib.nio.CloudStorageFileAttributesTest.testAcl Time elapsed: 0.451 s <<< ERROR!
2020-07-27T15:15:25.6302204Z java.nio.file.FileSystemNotFoundException: Provider "gs" not installed
2020-07-27T15:15:25.6310326Z at com.google.cloud.storage.contrib.nio.CloudStorageFileAttributesTest.before(CloudStorageFileAttributesTest.java:49)
2020-07-27T15:15:25.6311534Z

@elharo
Copy link
Contributor Author

elharo commented Jul 27, 2020

Just maybe relevant:

[WARNING] *****************************************************************
[WARNING] * Your build is requesting parallel execution, but project *
[WARNING] * contains the following plugin(s) that have goals not marked *
[WARNING] * as @threadsafe to support parallel building. *
[WARNING] * While this /may/ work fine, please look for plugin updates *
[WARNING] * and/or request plugins be made thread-safe. *
[WARNING] * If reporting an issue, report it against the plugin in *
[WARNING] * question, not against maven-core *
[WARNING] *****************************************************************
[WARNING] The following plugins are not marked @threadsafe in Storage Parent:
[WARNING] org.codehaus.mojo:clirr-maven-plugin:2.8
[WARNING] Enable debug to see more precisely which goals are not marked @threadsafe.
[WARNING] ***

@elharo
Copy link
Contributor Author

elharo commented Jul 27, 2020

tests are running in parallel and are not using random paths:

  @Before
  public void before() {
    CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.getOptions());
    path = Paths.get(URI.create("gs://bucket/randompath"));
    dir = Paths.get(URI.create("gs://bucket/randompath/"));
  }

@frankyn frankyn self-requested a review July 27, 2020 21:31
@frankyn frankyn added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 4, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 4, 2020
@suraj-qlogic suraj-qlogic added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 5, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 5, 2020
@elharo elharo added the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 11, 2020
@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/java-storage-nio API. label Aug 21, 2020
@frankyn frankyn removed the request for review from suraj-qlogic January 26, 2021 19:24
@elharo elharo requested a review from a team as a code owner January 26, 2021 21:16
@generated-files-bot
Copy link

generated-files-bot bot commented Jan 26, 2021

Warning: This pull request is touching the following templated files:

  • .kokoro/build.sh

@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 26, 2021
@frankyn
Copy link
Member

frankyn commented Jan 26, 2021

This issue might be related to recent failures. Not quite sure what's going on though as I'm not able to repro this issue locally.

@frankyn frankyn changed the title move autovalue to annotation processor path fix: move autovalue to annotation processor path Jan 26, 2021
@codecov
Copy link

codecov bot commented Jan 26, 2021

Codecov Report

Merging #179 (d153920) into master (1328de4) will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #179      +/-   ##
============================================
+ Coverage     72.21%   72.29%   +0.07%     
  Complexity      500      500              
============================================
  Files            29       29              
  Lines          1663     1664       +1     
  Branches        268      277       +9     
============================================
+ Hits           1201     1203       +2     
  Misses          336      336              
+ Partials        126      125       -1     
Impacted Files Coverage Δ Complexity Δ
...ge/contrib/nio/CloudStorageFileSystemProvider.java 63.23% <ø> (+0.32%) 76.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1328de4...375ff4d. Read the comment docs.

@frankyn
Copy link
Member

frankyn commented Jan 26, 2021

Woot, looks like things are passing now. Pending review from @BenWhitehead when he's back in the office tomorrow.

<path>
<groupId>com.google.auto.value</groupId>
<artifactId>auto-value</artifactId>
<version>1.7.3</version>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

1.7.4 is out now

@elharo
Copy link
Contributor Author

elharo commented Jan 27, 2021

com.google.api.client.http.HttpResponseException:
403 Forbidden

POST https://storage.googleapis.com:443/upload/storage/v1/b/bucket/o?uploadType=resumable&name=randompath
{
  "error": {
    "code": 403,
    "message": "it-service-account@gcloud-devel.iam.gserviceaccount.com does not have storage.objects.create access to bucket/randompath.",
    "errors": [
      {
        "message": "it-service-account@gcloud-devel.iam.gserviceaccount.com does not have storage.objects.create access to bucket/randompath.",
        "domain": "global",
        "reason": "forbidden"
      }
    ]
  }

@frankyn
Copy link
Member

frankyn commented Jan 27, 2021

Will continue debugging. This looks related to the open failures that exist right now.

@frankyn
Copy link
Member

frankyn commented Jan 27, 2021

@elharo I found that auto-service was missing in plugins which helped generate the META-INF/ file that's expected.

If it looks good to you, can we merge this in?

Copy link
Collaborator

@BenWhitehead BenWhitehead left a comment

Choose a reason for hiding this comment

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

Shared config has had auto-value support for some time.

We should be able to remove auto-value and auto-service and use the config from shared config.

See https://github.com/googleapis/java-firestore/pull/221/files for an example of it being used in Firestore.

Copy link
Contributor Author

@elharo elharo left a comment

Choose a reason for hiding this comment

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

@BenWhitehead PTAL. The commit history is becoming convoluted, but I think this is now what you asked for,

@elharo
Copy link
Contributor Author

elharo commented Jan 28, 2021

MockitoErrors in Java 7. CloudStorageLateInitializationTest.before:41 » UnsupportedClassVersion org/moc...

@frankyn
Copy link
Member

frankyn commented Jan 28, 2021

Stuck on:

[INFO] <<< maven-dependency-plugin:3.1.2:analyze (default-cli) < test-compile @ google-cloud-nio <<<
[INFO] 
[INFO] 
[INFO] --- maven-dependency-plugin:3.1.2:analyze (default-cli) @ google-cloud-nio ---
Warning:  Used undeclared dependencies found:
Warning:     com.google.auto.value:auto-value-annotations:jar:1.7.4:compile

Thanks @elharo!

@frankyn frankyn added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 28, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 28, 2021
@frankyn
Copy link
Member

frankyn commented Jan 28, 2021

Kokoro integrations failed again. I think it might be a flake and not sure how to reinforce without doing a retry of the tests.

@frankyn
Copy link
Member

frankyn commented Jan 28, 2021

Chatting with @BenWhitehead, the kokoro build change to retry tests will be removed by autosynth...

@frankyn
Copy link
Member

frankyn commented Jan 28, 2021

We will merge this PR once jobs pass and will follow-up with another solution provided by @BenWhitehead for flaky integration tests to separate tasks.
I'm still on the hook as part of On Duty next week.

@frankyn frankyn merged commit a5023f1 into master Jan 28, 2021
@frankyn frankyn deleted the av branch January 28, 2021 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage-nio API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Autovalue should be on the annotation processor classpath
5 participants