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 Jul 27, 2020
@google-cla google-cla bot added the cla: yes label Jul 27, 2020
@elharo
Copy link
Contributor Author

@elharo 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 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 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 Jul 27, 2020
@frankyn frankyn added the kokoro:force-run label Aug 4, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Aug 4, 2020
@suraj-qlogic suraj-qlogic added the kokoro:force-run label Aug 5, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Aug 5, 2020
@elharo elharo added the kokoro:run label Aug 11, 2020
@product-auto-label product-auto-label bot added the api: storage label Aug 21, 2020
@frankyn frankyn removed the request for review from suraj-qlogic Jan 26, 2021
@elharo elharo requested a review from as a code owner Jan 26, 2021
@generated-files-bot
Copy link

@generated-files-bot 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 label Jan 26, 2021
@frankyn
Copy link
Member

@frankyn 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 force-pushed the av branch 2 times, most recently from d4cd678 to 8caa011 Compare Jan 26, 2021
@frankyn frankyn requested a review from BenWhitehead Jan 26, 2021
@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 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 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

@elharo elharo Jan 27, 2021

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 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 frankyn commented Jan 27, 2021

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

@frankyn
Copy link
Member

@frankyn 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
Contributor

@BenWhitehead BenWhitehead left a comment

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

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

@elharo
Copy link
Contributor Author

@elharo elharo commented Jan 28, 2021

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

@frankyn
Copy link
Member

@frankyn 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 label Jan 28, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Jan 28, 2021
@frankyn
Copy link
Member

@frankyn 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 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 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
16 checks passed
@frankyn frankyn deleted the av branch Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants