Details
Diff Detail
- Repository
- rDWAPPS Web applications
- Branch
- arcpatch-D1339
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 5118 Build 6879: tox-on-jenkins Jenkins Build 6878: arc lint + arc unit
Event Timeline
Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/373/ for more details.
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 ? | |
Build has FAILED
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/374/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/374/console
Great! But you have to rebase that diff to origin/master first. That's why CI build is failing.
Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/375/ for more details.
Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/376/ for more details.
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. 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 | |
Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/377/ for more details.
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.