Depends on D3062.
Details
Details
- Reviewers
ardumont - Group Reviewers
Reviewers - Commits
- rDOBJS3043177f79b2: Normalize the handling of the config file loading in cli
Diff Detail
Diff Detail
- Repository
- rDOBJS Object storage
- Branch
- master
- Lint
Lint Skipped - Unit
Unit Tests Skipped - Build Status
Buildable 12048 Build 18274: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 18273: arc lint + arc unit
Event Timeline
Comment Actions
Build is green
Patch application report for D3063 (id=10896)
Could not rebase; Attempt merge onto 17f81a701e...
Updating 17f81a7..ca38cdf Fast-forward swh/objstorage/__init__.py | 107 ++------------------ swh/objstorage/api/server.py | 20 +++- swh/objstorage/cli.py | 21 +++- swh/objstorage/factory.py | 108 +++++++++++++++++++++ swh/objstorage/tests/test_multiplexer_filter.py | 2 +- swh/objstorage/tests/test_objstorage_api.py | 2 +- swh/objstorage/tests/test_objstorage_azure.py | 2 +- swh/objstorage/tests/test_objstorage_in_memory.py | 2 +- .../tests/test_objstorage_instantiation.py | 2 +- .../tests/test_objstorage_multiplexer.py | 2 +- .../tests/test_objstorage_pathslicing.py | 4 +- .../tests/test_objstorage_random_generator.py | 2 +- swh/objstorage/tests/test_objstorage_striping.py | 2 +- 13 files changed, 158 insertions(+), 118 deletions(-) create mode 100644 swh/objstorage/factory.py
Changes applied before test
commit ca38cdf58b472582fe96aed1da453753376649a7
Author: David Douard <david.douard@sdfa3.org>
Date: Fri Apr 24 15:44:18 2020 +0200
Normalize the handling of the config file loading in cli
commit 68d01a09251b18de3c6a865d7c2fcbe0b1a6af8f
Author: David Douard <david.douard@sdfa3.org>
Date: Fri Apr 24 15:39:35 2020 +0200
Move the content of swh/objstorage/__init__.py in swh/objstorage/factory.py
This is needed to "empty" the swh.objstorage's *module* from any
non-subpackage objects, so we can implement the swh.objstorage.replayer
outside the swh.objstorage's repo, in a dedicated repo/project.See https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/4/ for more details.
| swh/objstorage/api/server.py | ||
|---|---|---|
| 237–238 | doesn't need to return it | |
| swh/objstorage/api/server.py | ||
|---|---|---|
| 237–238 | I do prefer the fact the validation function returns the validated config value. It makes, for example, the user code cleaner (but it's a matter of taste I guess): app = make_app(validate_config(ctx.obj["config"])) vs. validate_config(ctx.obj["config"]) app = make_app(ctx.obj["config"]) | |
| swh/objstorage/api/server.py | ||
|---|---|---|
| 227 | Well, I did hesitate doing so, but the proper type is a bit a pain to express, so it would be just a Dict[Any] which is not really accurate, so... | |
| swh/objstorage/api/server.py | ||
|---|---|---|
| 237–238 | I do prefer in general function (returns something) other procedure (does not return). But in any case, for your point, returning the validating config sounds like something easier to reason about so +1 ;) | |
| swh/objstorage/api/server.py | ||
|---|---|---|
| 227 | Dict[str, Any]? yes, it's not much, but at least contributors know immediately it's a dict without looking elsewhere. | |
| swh/objstorage/api/server.py | ||
|---|---|---|
| 227 | I mean, the proper type should be defined in swh.core.config IMHO | |
| swh/objstorage/api/server.py | ||
|---|---|---|
| 237–238 | /me sighs at his typos he cannot fix...
over* procedure (as in c language) | |
| swh/objstorage/api/server.py | ||
|---|---|---|
| 227 | Dict[Any] is a shortcut for Dict[str, Any] IIRC. | |
| swh/objstorage/api/server.py | ||
|---|---|---|
| 227 | ok then, don't worry about it (i won't ;) That's plenty good as is already, thus why it's accepted. | |