Diff Detail
- Repository
- rDWAPPS Web applications
- Branch
- vault
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 923 Build 1225: Software Heritage Python tests Build 1224: arc lint + arc unit
Event Timeline
This seems quite fine but it misses tests.
Those help quite a lot in understanding what happens for non-optimal use cases.
Hint:
If you don't want to add those to every layer (api, service, backend) which can be a hassle (don't i know it :).
You could test only the api layer, mocking only the last backend layer.
| swh/web/ui/main.py | ||
|---|---|---|
| 68 | It's not an obligation but maybe defining a get_vault function in swh.vault.__init__ the same way we define for swh.storage (get_storage) or swh.objstorage (get_objstorage) would be good as well. This would be more composable (choosing between a remote vault or local one) and symmetric in configuration (in regards to storage). | |
| swh/web/ui/views/api.py | ||
| 1156 | What happens if res is not optimal because dir_id does not exist for example? Would what's following return an empty file? | |
| swh/web/ui/main.py | ||
|---|---|---|
| 68 | Huh, the vault isn't designed to have a "local version" as of yet. But it should be pretty easy. | |
| swh/web/ui/main.py | ||
|---|---|---|
| 68 |
My bad then, I thought it had :) | |
| swh/web/ui/views/api.py | ||
|---|---|---|
| 1156 | < HTTP/1.0 404 NOT FOUND
< Content-Type: application/json
[.. blah blah ..]
{"exception": "NotFoundExc", "reason": "Directory with ID 'd4905454cc154dddfddded48694ae3c579345e' not found."} | |
Beware, the tests failed. I think you forgot to push your latest development on swh-vault.
The errors refer to an unknown swh.vault.api.client.RemoveVaultClient (reproduced locally).
At current swh-vault's HEAD, i have a swh.vault.api.client.RemoteVaultCache.
Hiding those details inside the swh.vault.get_vault i proposed earlier would have made the tests ok :)
| requirements-swh.txt | ||
|---|---|---|
| 4 | We need to duplicate that information in debian/control file as well at least in the Build-Depends (Due to a shortcoming of the setup.py routine IIRC): python3-swh.vault (>= 0.0.1~), (no trailing comma if it's the last line :). | |
| swh/web/ui/views/api.py | ||
| 1156 | Right, now i see it :) | |
In my opinion, views api_vault_cook_directory and api_vault_cook_revision_gitfast should be merged into a single one as their content is quite redundant (idem for api_vault_fetch_directory and api_vault_fetch_revision_gitfast).
I attach a Paste https://forge.softwareheritage.org/P176 to show how it could be done using the new version of the swh web api based on django rest framework.
| swh/web/ui/main.py | ||
|---|---|---|
| 102 | small docstring issue here | |
Needs to be adapted to the new web ui interface (service/backend split doesn't exist anymore, etc)