Skip to content

Moving MD5 hash method from regression to storage helpers. - #611

Merged
dhermes merged 2 commits into
googleapis:masterfrom
dhermes:move-md5-to-library
Feb 12, 2015
Merged

Moving MD5 hash method from regression to storage helpers.#611
dhermes merged 2 commits into
googleapis:masterfrom
dhermes:move-md5-to-library

Conversation

@dhermes

Copy link
Copy Markdown
Contributor

May make sense to swap out for hashlib.md5 (though they seem to
be equivalent).

In preparation for #547.

@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 10, 2015
@dhermes

Copy link
Copy Markdown
ContributorAuthor

If we really want to do this, we'll need to support computing MD5 of files that don't fit in memory:
http://stackoverflow.com/a/1131255/1068170

This is done in a generic way in gsutil (though the method is quite a mouthful).

@dhermes

Copy link
Copy Markdown
ContributorAuthor

@tseaver The lint failure is a bug in my PR speedup code since among

gcloud/storage/_helpers.py
gcloud/storage/test__helpers.py
regression/storage.py

the abstract class is really never used.

We have 3 options:

  • I make a temporary commit disabling the PR speedup to just diff against all the files
  • I try to find a full-time fix (done in Repeating pylint after failures. #612)
  • Ignore and make sure it passes when merged

dhermes added a commit to dhermes/google-cloud-python that referenced this pull request Feb 10, 2015
This surfaced as a false negative in googleapis#611.
@tseaver

Copy link
Copy Markdown
Contributor

LGTM. I'd say we should probably not set the envvars on Travis, to avoid these kinds of errors.

@dhermes

Copy link
Copy Markdown
ContributorAuthor

We don't set the env vars on merges to master. This only happens for PRs, which we want to be quick and snappy during review.

Comment threadgcloud/storage/_helpers.py Outdated

This comment was marked as spam.

This comment was marked as spam.

@dhermes

Copy link
Copy Markdown
ContributorAuthor

Went with 8192 for the chunk size since gsutil does:
https://github.com/GoogleCloudPlatform/gsutil/blob/bc083d405b92856a1aba46a65410bd1a1dbf65a6/gslib/util.py#L66

@tseaver PTAL

I also made a separate method for updating a hash object for when CRC32 comes around. (Sort of YAGNI, sort of not.)

@tseaver

Copy link
Copy Markdown
Contributor

@dhermes pylint failure due to partial-set checking.

@dhermes

Copy link
Copy Markdown
ContributorAuthor

Yes, recall that this PR is the reason for #612.

May make sense to swap out for `hashlib.md5` (though they seem to
be equivalent).
In preparation for googleapis#547.
This is to account for files too large to fit into memory.
@dhermes

Copy link
Copy Markdown
ContributorAuthor

@tseaver I just rebased on top of #612.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling 86b1f59 on dhermes:move-md5-to-library into df4c625 on GoogleCloudPlatform:master.

@tseaver

Copy link
Copy Markdown
Contributor

LGTM.

dhermes added a commit that referenced this pull request Feb 12, 2015
Moving MD5 hash method from regression to storage helpers.
@dhermes
dhermes merged commit c88ba72 into googleapis:masterFeb 12, 2015
@dhermes

Copy link
Copy Markdown
ContributorAuthor

It seems coveralls has recovered from whatever was making it silently fail last night.

@dhermes
dhermes deleted the move-md5-to-library branch February 12, 2015 19:32
@dhermesdhermes mentioned this pull request Feb 14, 2015
vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
* docs: Add documentation for enums
fix: Add context manager return types
chore: Update gapic-generator-python to v1.8.1
PiperOrigin-RevId: 503210727
Source-Link: googleapis/googleapis@a391fd1
Source-Link: googleapis/googleapis-gen@0080f83
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMDA4MGY4MzBkZWMzN2MzMzg0MTU3MDgyYmNlMjc5ZTM3MDc5ZWE1OCJ9
* 🦉 Updates from OwlBot post-processor
See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Aug 21, 2025
parthea pushed a commit that referenced this pull request Sep 16, 2025
parthea pushed a commit that referenced this pull request Nov 22, 2025
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Nov 24, 2025
Source-Link: googleapis/synthtool@69fabae
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:562802bfac02e012a6ac34eda282f81d06e77326b82a32d7bbb1369ff552b387
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Nov 25, 2025
Source-Link: googleapis/synthtool@69fabae
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:562802bfac02e012a6ac34eda282f81d06e77326b82a32d7bbb1369ff552b387
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Mar 6, 2026
Add unit tests for many spanner_django modules that add coverage beyond the built-in django tests.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@dhermes@tseaver@coveralls@googlebot