-
Notifications
You must be signed in to change notification settings - Fork 20
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
Fixes cheksum failure when :md5sum option is a URL. #114
Closed
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
I think that maybe there is a missing |
ok, i added the missing method in order to keep backwards compatibility |
dkinzer
added a commit
to tulibraries/tul_cob
that referenced
this pull request
Jan 26, 2018
Even though the solr_wrapper docs say the mad5sum takes a URL to an md5sum or a file path, it actual fails in those cases. Looking at the code it will work fine if we pass the actual md5sum here (I think). I made a PR against solr_wrapper to fix the issue I found: cbeer/solr_wrapper#114
dkinzer
changed the title
Fixes cheksum failure when mdsum is URL.
Fixes cheksum failure when :m5dsum option is a URL.
Jan 27, 2018
dkinzer
changed the title
Fixes cheksum failure when :m5dsum option is a URL.
Fixes cheksum failure when :md5sum option is a URL.
Jan 27, 2018
dkinzer
force-pushed
the
fix-mdsum-fail
branch
from
January 28, 2018 20:44
831f6d3
to
f42bd1b
Compare
When the :md5sum option is a URL either it is treated as an mdsum string failing checksum test or it is not used and default mdsum url for default mirror is used. In either case there is a check sum failure. This commit fixes that issue by overriding the default md5url when mdsum is a url. This commit also add some tests for this bug fix as well as fixes a bug in the download URL code that throws an error when a file request is stubbed.
dkinzer
force-pushed
the
fix-mdsum-fail
branch
from
January 28, 2018 20:46
f42bd1b
to
eb53ad1
Compare
skng5
pushed a commit
to tulibraries/tul_cob
that referenced
this pull request
Jan 29, 2018
* Refactor sort electronic url * Fixes bug where an empty accumulater gets clobbered. * Removes unused fixtures. * Adds smoke test to verify code sorts as expected. * Adds test against bug * Update solr-wrapper download url. * Update solr zip url via params. * Update md5sum url. * Add md5url not md5sum. * Use md5sum instead of URL to md5sum file. Even though the solr_wrapper docs say the mad5sum takes a URL to an md5sum or a file path, it actual fails in those cases. Looking at the code it will work fine if we pass the actual md5sum here (I think). I made a PR against solr_wrapper to fix the issue I found: cbeer/solr_wrapper#114 * Use valid checksum
dkinzer
pushed a commit
to tulibraries/tul_cob
that referenced
this pull request
Jan 30, 2018
* List in descending chronological order * Intermediate commit - Extend sorting - Breaks out sort function into separate method - Extends sort to end date and title * Fine tune sort and place make it a separate method * Fix tertiary sort - Teritary sort on title instead of ID and title * Remove unused fixture file * Refactor sort electronic url (#221) * Refactor sort electronic url * Fixes bug where an empty accumulater gets clobbered. * Removes unused fixtures. * Adds smoke test to verify code sorts as expected. * Adds test against bug * Update solr-wrapper download url. * Update solr zip url via params. * Update md5sum url. * Add md5url not md5sum. * Use md5sum instead of URL to md5sum file. Even though the solr_wrapper docs say the mad5sum takes a URL to an md5sum or a file path, it actual fails in those cases. Looking at the code it will work fine if we pass the actual md5sum here (I think). I made a PR against solr_wrapper to fix the issue I found: cbeer/solr_wrapper#114 * Use valid checksum * Rescue from bad record on sort.
This was referenced Sep 6, 2018
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.
According to the current docs
md5sum
is either a path or URL to a filecontaining an md5sum.
When the
md5sum
is a URL either it is treated as an md5sum string failingchecksum test (because it's a URL) or it is not used and default md5sum url for
the default mirror is used instead. In either case there is a check sum failure.
This commit fixes that issue by overriing the default md5url when
md5sum
is a url.