Page MenuHomeSoftware Heritage

Add more redirections for upper case sha1
ClosedPublic

Authored by kalpitk on Apr 3 2019, 2:57 PM.

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

anlambert requested changes to this revision.EditedApr 3 2019, 3:45 PM

Looks good so far. Nevertheless, there is still missing redirections for the following endpoints:

  • /api/1/content/*
  • /api/1/release
  • /api/1/revision
  • /api/1/revision/directory
  • /api/1/revision/log
  • /api/1/revision/raw
  • /api/1/snapshot
  • /browse/content/*

For the content endpoints, it is a bit particular as we enable to pass the hashing algorithm name before the checksum (if not provided, the checksum is considered as a sha1)
(for instance: https://archive.softwareheritage.org/browse/content/blake2s256:9589a8d5f6b53a7be06253c3789019592fd6f3b442f69e98deedd6014201b43c/).

In order for the redirection mechanism to work you will have to update the route descriptions by updating the regexps for the query_string url argument (q for the api)
from .+ to [0-9a-z_:]*[0-9a-f]+.

swh/web/common/urlsindex.py
41

Can you add a docstring explaining why we put these url redirections in place ?

This revision now requires changes to proceed.Apr 3 2019, 3:45 PM
  • Add uppercase redirections for APIs and tests

Great! But you have to rebase that diff to origin/master first. That's why CI build is failing.

Looks good! Can you address my last comments and I will accept that diff.

swh/web/api/views/content.py
19โ€“20

add a line break before 'api-content-filetype' to avoid the use of #noqa

64โ€“65

same here

swh/web/common/urlsindex.py
41

docstring please

This revision now requires changes to proceed.Apr 4 2019, 5:01 PM

Still a couple of modifications to add to the docstring.

Once it is done, can you squash the commits and add the following commit message:

Add support for upper/mixed case checksum url arguments

Closes T1505
swh/web/common/urlsindex.py
44

when upper/mixed case checksums are passed as url arguments.

46

The method does not return anything so you can remove that part.

Nevertheless, method arguments should be documented.
Just copy/paste the following text:

Args:
    view_name (str): name of the view to redirect requests
    url_patterns (List[str]): regexps describing the view urls
    checksum_args (List[str]): url argument names corresponding to checksum values
This revision now requires changes to proceed.Apr 4 2019, 7:33 PM

Add redirections for upper/mixed case checksum

This revision is now accepted and ready to land.Apr 4 2019, 9:15 PM

Nice work, thanks! You should now be able to land the revision by yourself by clicking on the "Land revision" link on the top right part of that page.

This revision was automatically updated to reflect the committed changes.