Page MenuHomeSoftware Heritage

fix snapshot for upper case SHA1
AbandonedPublic

Authored by kalpitk on Mar 22 2019, 8:09 AM.

Details

Summary

url with sha1 in uppercase (or mixed upper-lower case) was gives 404.

Convert snapshot_id to snapshot_id.lower(), so that SHA1 remains lowercase consistently. I will also work with SHA1 with mixed upper and lower cases.

Related T1505

Currently passing a sha1 as url argument to the webapp endpoints works only if it is in lowercase form.
If a sha1 is passed in uppercase form, we end up in a 404 error page (see [1] for instance).

For commodity of use, we should support passing sha1s in uppercase form but also with
mixed uppercase and lowercase parts.

Redirecting to the lowercase sha1 endpoints when such cases are encountered seems the
right way to implement this.

[1] https://archive.softwareheritage.org/browse/snapshot/1A8893E6A86F444E8BE8E7BDA6CB34FB1735A00E/

Diff Detail

Repository
rDWAPPS Web applications
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 4766
Build 6349: tox-on-jenkinsJenkins

Event Timeline

First, thanks for the contribution.

@kalpitk Can you please title the diff appropriately?

I don't know off-hand what the T1505 is all about, so i need to either open the task for the description (hoping it's described) or look it up in the forge.
While you can refer to it rather simply in the description (adding the following snippet in the diff's description):

Related T1505

Can you please adapt accordingly?

Thanks in advance.


Other than that, that sounds good.
I've triggered back the build as the failure seems unrelated.

Cheers,

Also, please adapt appropriate the commit message.

You can find the documentation about code review [1] (following the links mentioned is a good idee, the git style convention notably here).

[1] https://wiki.softwareheritage.org/wiki/Code_review_in_Phabricator#Starting_a_new_feature_and_submit_it_for_review

Again, thanks.

In the mean time, i'm trying to understand what's wrong with the ci failure.

Cheers,

This revision now requires changes to proceed.Mar 22 2019, 2:40 PM

Fyi, patch is fine locally.

I've referenced T1597 to try and investigate further (i tried to but did not see anything shockingly wrong) why diff is red.

kalpitk retitled this revision from fixes T1505 to fix snapshot for upper case SHA1.Mar 22 2019, 7:19 PM
kalpitk edited the summary of this revision. (Show Details)

@ardumont Changed the title.

I didn't know about working with differentials earlier, so I had used web interface to submit the diff. And there was no option to add commit message there.
Next time I'll be using arc cli.

Anyways, I have a feeling that using web interface (for uploading diff) messed things. Jenkins doesn't even show the changes in this build - https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/342/changes

I pushed changes through arc : D1290
Probably the problem was line 34, which had more than 79 characters. This was preventing me to stage changes.

@kalpitk , as you opened D1290, you should abandon this diff to avoid duplicate.