cleanup(storage): use Disable{MD5*,Crc32c*} consistently#7034
Merged
coryan merged 1 commit intogoogleapis:mainfrom Jul 20, 2021
Merged
cleanup(storage): use Disable{MD5*,Crc32c*} consistently#7034coryan merged 1 commit intogoogleapis:mainfrom
coryan merged 1 commit intogoogleapis:mainfrom
Conversation
All GCS options are implemented as an `absl::optional<T>`, in the case of `DisableMD5Hash` and `DisableCrc32cChecksum` this is `absl::optional<bool>`. The MD5 hashes are disabled by default, this is achieved by having the default constructor for `DisableMD5Hash` initialize the option to `true`. CRC32C checksums are enabled by default, the default constructor initializes the optional to `absl::nullopt` (this is consistent with all the other options). After this change we always use these options by doing: ```cc auto value = request.GetOption<Disable*>().value_or(false); ``` This makes the usage consistent, and only the constructor encodes what the default is.
product-auto-label
Bot
added
the
api: storage
googlebot
added
the
cla: yes
Contributor
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #7034 +/- ##
==========================================
- Coverage 94.47% 94.47% -0.01%
==========================================
Files 1305 1305
Lines 113414 113412 -2
==========================================
- Hits 107148 107144 -4
- Misses 6266 6268 +2
Continue to review full report at Codecov.
|
devjgm
reviewed
Jul 20, 2021
Contributor
devjgm
left a comment
devjgm
left a comment
There was a problem hiding this comment.
Thanks. Just one question.
devjgm
approved these changes
Jul 20, 2021
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
All GCS options are implemented as an
absl::optional<T>, in the caseof
DisableMD5HashandDisableCrc32cChecksumthis isabsl::optional<bool>. The MD5 hashes are disabled by default, this isachieved by having the default constructor for
DisableMD5Hashinitialize the option to
true. CRC32C checksums are enabled bydefault, the default constructor initializes the optional to
absl::nullopt(this is consistent with all the other options).After this change we always use these options by doing:
This makes the usage consistent, and only the constructor encodes
what the default is.
Fixes #7029
This change is