Add Google Cloud Storage NIO Alpha support #1090
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mziccard
added
do not merge
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
googlebot
added
the
cla: no
| * Resuming upload or download | ||
| * Generations | ||
| * File attributes | ||
| * (more) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
A few minor comments, but otherwise looks good to me! Thank you @mziccard for doing this! |
|
Why are you rebasing? Rebasing destroys the history of review. |
To avoid merging the ton of merge commits we made form master to gcs-nio back into master. To make history more linear and easier to browse through it. |
This is an import of previous work which was reviewed internally.
Also implemented FakeStorageRPC to provide an in-memory GCS implementation for testing (it's not complete yet, but good enough for simple tests).
I also made the following additional changes: - Don't use one-line javadocs, per request of @aozarov - Use public domain John Keats poetry published in 1819 - Minor cleanup to a few JavaDocs
This change is a follow-up to post-merge comments in googleapis#706 made by @ajkannan and @aozarov. The following changes have been made: - Don't use @nullable on equals() methods - Don't `throws Exception` in test methods unless needed - Use incomplete sentences in JavaDoc summary fragment - Capitalize Cloud Storage - Get rid of some one-line JavaDocs - Use @code formatting when appropriate - Remove Paths.get() usages which is deprecated in Java 8 - Remove @SuppressWarnings("null") which no one uses
* Split off the "normal case" example from the "legacy case" one. (and create a "normal case" example, first) * Correct damage from "helpful" IDE renaming help. * Switch gcloud-java-examples to appassembler This has the following benefits: * Stat example now works * Easier to see what is available (list the bin folder) * Easier to see what running an example entails (read the script) * Simpler command line (no more -Dexec.args) * Rename example to examples and switch a Stat to ListFilesystems * Clean up pom Fix indent to 2, remove redundant groupId line that's inherited from the parent. * Reviewer comments and update_docs_version.sh * Added blank line * Sweating the small stuff Adding a \n at the end of the last line.
Fix javadoc in examples to match the README Also use "log in" instead of "login" where a verb is needed instead of a noun.
* Add url to the pom for nio and nio-examples To mirror the changes in master. * Remove already-updated line
- Stat is now in com.google.cloud.examples.nio (following the gcloud -> cloud move) - Added version for appassembler to remove warning - added 'mvn install' to instructions because it's necessary.
Made it and CountBytes compute an MD5, so I could check that they match (they do).
* Handle files over 2GB * Use Guava's checkArgument
Documentation has been added to many methods to clarify the preferred method for instantiating the library, which is the non-SPI version.
mziccard
force-pushed
the
add-nio-support
branch
from
acd4272
to
2f231b3
Compare
mziccard
force-pushed
the
add-nio-support
branch
from
2f231b3
to
0bef38d
Compare
|
@jean-philippe-martin I added two commits to fix your comments and a bunch of minor checkstyle, codacy and docs issues. I believe that codacy's issues left can be ignored. PTAL |
|
Looks good to me! |
| * | ||
| * class MyTest { | ||
| * {@literal @}Rule | ||
| * public final AppEngineRule appEngine = AppEngineRule.builder().build(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
mziccard
force-pushed
the
add-nio-support
branch
2 times, most recently
from
ea5f856
to
7b35876
Compare
mziccard
removed
the
do not merge
github-actions bot
pushed a commit
that referenced
this pull request
Oct 4, 2022
…#1090) [](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.cloud:google-cloud-core](https://github.com/googleapis/java-core) | `2.8.18` -> `2.8.19` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>googleapis/java-core</summary> ### [`v2.8.19`](https://github.com/googleapis/java-core/blob/HEAD/CHANGELOG.md#​2819-httpsgithubcomgoogleapisjava-corecomparev2818v2819-2022-09-30) [Compare Source](https://github.com/googleapis/java-core/compare/v2.8.18...v2.8.19) ##### Dependencies - Update dependency com.google.api.grpc:proto-google-common-protos to v2.9.5 ([#​963](https://github.com/googleapis/java-core/issues/963)) ([1ee2f9a](https://github.com/googleapis/java-core/commit/1ee2f9acd3968b21974a550b6732fe68b18085f4)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-translate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yMDguMiIsInVwZGF0ZWRJblZlciI6IjMyLjIwOC4yIn0=-->
github-actions bot
pushed a commit
that referenced
this pull request
Oct 5, 2022
](https://renovatebot.com){kind=link}
🤖 I have created a release *beep* *boop* --- ## [2.3.5](https://github.com/googleapis/java-translate/compare/v2.3.4...v2.3.5) (2022-10-03) ### Dependencies * Update dependency com.google.cloud:google-cloud-core to v2.8.12 ([#1076](https://github.com/googleapis/java-translate/issues/1076)) ([d76889c](https://github.com/googleapis/java-translate/commit/d76889cc92fb1cad71d4be4552d68c962deddfed)) * Update dependency com.google.cloud:google-cloud-core to v2.8.13 ([#1083](https://github.com/googleapis/java-translate/issues/1083)) ([3819209](https://github.com/googleapis/java-translate/commit/3819209e110df183aba335e22b404c6bc415772a)) * Update dependency com.google.cloud:google-cloud-core to v2.8.14 ([#1084](https://github.com/googleapis/java-translate/issues/1084)) ([c2e0417](https://github.com/googleapis/java-translate/commit/c2e04178b37396435f6c24d180fd67cbf9bfba3c)) * Update dependency com.google.cloud:google-cloud-core to v2.8.15 ([#1085](https://github.com/googleapis/java-translate/issues/1085)) ([554b02f](https://github.com/googleapis/java-translate/commit/554b02fa2b97ff4841ae35e7e43ab6c72335a97e)) * Update dependency com.google.cloud:google-cloud-core to v2.8.18 ([#1087](https://github.com/googleapis/java-translate/issues/1087)) ([dc3663e](https://github.com/googleapis/java-translate/commit/dc3663e29574467bd25400c7d91bd0c1ca036038)) * Update dependency com.google.cloud:google-cloud-core to v2.8.19 ([#1090](https://github.com/googleapis/java-translate/issues/1090)) ([982faf0](https://github.com/googleapis/java-translate/commit/982faf0eba19064a3b82ad45c2567e613aa225e9)) * Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.4 ([#1092](https://github.com/googleapis/java-translate/issues/1092)) ([34fd42e](https://github.com/googleapis/java-translate/commit/34fd42e19a494aebfd1f6ab73274ed5b38f175a4)) * Update dependency com.google.cloud:google-cloud-storage to v2.12.0 ([#1078](https://github.com/googleapis/java-translate/issues/1078)) ([06861c1](https://github.com/googleapis/java-translate/commit/06861c153ec52be12b8d2e891482bdef76f0a850)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
github-actions bot
pushed a commit
that referenced
this pull request
Oct 10, 2022
…rm to v3.4.1 (#1090) [](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.cloud:google-cloud-aiplatform](https://github.com/googleapis/java-aiplatform) ([source](https://github.com/googleapis/java-)) | `3.4.0` -> `3.4.1` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-aiplatform). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yMzAuMCIsInVwZGF0ZWRJblZlciI6IjMyLjIzMC4wIn0=-->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR takes the
gcs-niobranch and rebases it on top of master. All redundant commits are removed.acd4272 updates docs, poms and REAMDEs to use appassembler for pubsub's example.
@jart @jean-philippe-martin can you have a look at this? All code (but acd4272) is already reviewed so just a quick look should be enough to spot possible incosistencies/errors left. Please do not merge this as I need to merge other PRs into master before this can go through.