Page MenuHomeSoftware Heritage
Differential D32

Add /revision/<sha1_git>/raw api route
ClosedPublic
Actions

Authored by jbertran on May 26 2016, 4:29 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T298: Add /api/1/revision/<sha1_git>/raw/
Commits
R65:1add258a7f99: api: use raw backend lookup instead of service wrapper for raw rev msg
R65:cd6906154b9e: tests: update api and service tests to reflect changes
R65:6f45b82fd3e6: Service: more idiomatic dictionary removal
R65:7a0cfff3f9a2: api tests: test exception message reflects the error it should report
R65:a5b1e3fe9f63: service and api tests: fix raw revision message tests
R65:4f22178ad32c: api view: added /api/1/revision/<sha1_git>/raw/ route
R65:ca51fbaab71a: test_api: update to reflect the addition of /api/1/revision/<sha1_git>/raw to…
R65:96bfc8db9daf: api: update lookup_revision to reflect service changes, add raw message lookup
R65:9c7cd094c321: service: * add function to get _only_ raw revision message * restrict…
R65:476cf9275bc2: Converters: fixed a bug where the revision author's full name was not decoded…
rDWAPPS9c7cd094c321: service: * add function to get _only_ raw revision message * restrict…
rDWAPPS96bfc8db9daf: api: update lookup_revision to reflect service changes, add raw message lookup
rDWAPPS7a0cfff3f9a2: api tests: test exception message reflects the error it should report
rDWAPPSa5b1e3fe9f63: service and api tests: fix raw revision message tests
rDWAPPScd6906154b9e: tests: update api and service tests to reflect changes
rDWAPPSca51fbaab71a: test_api: update to reflect the addition of /api/1/revision/<sha1_git>/raw to…
rDWAPPS1add258a7f99: api: use raw backend lookup instead of service wrapper for raw rev msg
rDWAPPS6f45b82fd3e6: Service: more idiomatic dictionary removal
rDWAPPS4f22178ad32c: api view: added /api/1/revision/<sha1_git>/raw/ route
rDWAPPS476cf9275bc2: Converters: fixed a bug where the revision author's full name was not decoded…
Summary

converters: fixed a bug where /revision/<sha1_git> would raise an exception
api view: added /api/1/revision/<sha1_git>/raw/ route
test_api: update to reflect the addition of /api/1/revision/<sha1_git>/raw to api routes.

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

jbertran updated this revision to Diff 99.May 26 2016, 4:29 PM
jbertran added a task: T298: Add /api/1/revision/<sha1_git>/raw/.
jbertran retitled this revision from to Add /revision/<sha1_git>/raw api route.
jbertran updated this object.
jbertran edited the test plan for this revision. (Show Details)
Herald added a reviewer: Reviewers. · View Herald TranscriptMay 26 2016, 4:29 PM
ardumont added a subscriber: ardumont.May 26 2016, 4:48 PM
ardumont added inline comments.
swh/web/ui/converters.py
167

Yes, indeed.
The base api got updated on person and fullname got added.

swh/web/ui/views/api.py
516 ↗(On Diff #99)

I think the point was to make the actual api_revision returns the full revision except for its message.
(so you should not call it here).

And this new api should only return the revision message.

And in the views, the browse revision should:

  • display the revision for all fields except for the revision message
  • for the revision message, a link to another view which calls for the raw content

All of this for avoiding trouble when revision message are not utf-8 encoded.

jbertran added inline comments.May 26 2016, 4:55 PM
swh/web/ui/views/api.py
516 ↗(On Diff #99)

Gotcha.

I'll update the revision and revision/raw tests to reflect that as well, the stubs included messages, hence my confusion.

ardumont added inline comments.May 26 2016, 5:05 PM
swh/web/ui/views/api.py
516 ↗(On Diff #99)

hence my confusion

Well, the description does not say anything about this, so it's not like you could have foreseen it
^^

ardumont requested changes to this revision.May 30 2016, 9:40 AM
ardumont added a reviewer: ardumont.
This revision now requires changes to proceed.May 30 2016, 9:40 AM
jbertran updated this revision to Diff 108.May 30 2016, 11:18 AM
jbertran edited edge metadata.

Split revision/raw message lookups

  • service: add raw message only lookup
  • api: update lookup_revision to reflect service changes, add raw
  • tests: update api and service tests to reflect changes
ardumont requested changes to this revision.May 30 2016, 1:26 PM
ardumont edited edge metadata.
ardumont added inline comments.
swh/web/ui/tests/views/test_api.py
744 ↗(On Diff #108)

As we said earlier, it should be None here instead.
To be compliant with your test function name that is ^^

753 ↗(On Diff #108)

...And you'll get a 404 here.

swh/web/ui/views/api.py
520 ↗(On Diff #108)

As we exchange, you can simply call service.lookup_revision_message since it does the job already.

This revision now requires changes to proceed.May 30 2016, 1:26 PM
jbertran updated this revision to Diff 109.May 30 2016, 1:45 PM
jbertran edited edge metadata.

Minor corrections

  • Service: more idiomatic dictionary removal
  • api: use raw backend lookup instead of service wrapper for raw rev msg
  • service and api tests: fix raw revision message tests
ardumont added inline comments.May 30 2016, 1:54 PM
swh/web/ui/tests/views/test_api.py
768 ↗(On Diff #109)

Nitpick, it's the 'no revision was found' use case.

jbertran added inline comments.May 30 2016, 1:56 PM
swh/web/ui/tests/views/test_api.py
768 ↗(On Diff #109)

You'd think that was a misplaced copy-paste, but it was hand-written the same in both tests --'

jbertran updated this revision to Diff 110.May 30 2016, 1:58 PM
jbertran edited edge metadata.

Fix error message

  • api tests: test exception message reflects the error it should report
ardumont accepted this revision.May 30 2016, 1:59 PM
ardumont edited edge metadata.
ardumont added inline comments.
swh/web/ui/tests/views/test_api.py
768 ↗(On Diff #109)

oops ^^

This revision is now accepted and ready to land.May 30 2016, 1:59 PM
Closed by commit rDWAPPS476cf9275bc2: Converters: fixed a bug where the revision author's full name was not decoded… (authored by jbertran). · Explain WhyMay 30 2016, 2:05 PM
This revision was automatically updated to reflect the committed changes.
zack edited edge metadata.May 30 2016, 3:21 PM
zack changed the visibility from "All Users" to "Public (No Login Required)".

Revision Contents
Changeset List

PathSize
swh/
web/
ui/
1 line

Diff 111

View Options

swh/web/ui/converters.py

Loading...
About · Developer information · Legal