swh.deposit.cli: Transform create_user into a main cli (+ docs updates)
ClosedPublic
Actions

Authored by ardumont on Feb 24 2019, 9:46 AM.

Details

Reviewers

douardda

Group Reviewers

Reviewers

Commits

rDDEP0545d93e390f: swh.manage: Migrate manage to swh/deposit subfolder
rDDEP57f54dcc2b5a: settings.production: Improve error message when missing information
rDDEP992f64cac9c5: docs/sys-info: Update user creation routine documentation
rDDEP62af09bbffd7: dev-info: Update according to latest configuration change
rDDEP2e932c405b02: swh.deposit.cli: Add list subcommands (user, collection)
rDDEP515aedeba076: swh.deposit.cli: Document a tad further entrypoints
rDDEP09dbcca9f4ea: swh-deposit user create: Create user's collection if need be
rDDEP3ec8840e832a: swh.deposit.cli: Add collection subtask
rDDEP92ebfa68bd40: setup.py: Expose the 2 cli entrypoints and avoid the clash names
rDDEP77a36c4bfa25: swh.deposit.cli: Use simple click.echo call
rDDEPce40f37345d4: swh.deposit.cli: Transform create_user into a main cli

Summary

Improve the user create subcommand to create the collection if need be (instead of stopping and doing an extra command ~> tedious).
Improve the user subcommand cli to allow basic listing
Open cli subtask to manipulate collections (list, create) as well
docs/sys-info: Update user creation routine documentation
dev-info: Update according to latest configuration change

Related T1580

Test Plan

tox

Diff Detail

Repository

rDDEP Push deposit

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Feb 24 2019, 9:46 AM

Build is green
See https://jenkins.softwareheritage.org/job/DDEP/job/tox/85/ for more details.

Harbormaster completed remote builds in B4418: Diff 3727.Feb 24 2019, 9:48 AM

ardumont marked an inline comment as done.Feb 24 2019, 9:51 AM

ardumont added inline comments.

setup.py
56 ↗	(On Diff #3727)	context: Deposit is split in 3 debian packages (python3-swh.deposit, python3-swh.deposit.client, python3-swh.deposit.loader) so this will be installed broken for some of those (i think). The swh-deposit-client cli is packaged in a dedicated debian package so i expect this to not be broken for that package only (i could be wrong). @douardda In any case, it might be worth considering to merge that in the main cli?

settings.production: Improve error message when missing information

ardumont marked an inline comment as done.Feb 24 2019, 10:03 AM

ardumont added inline comments.

docs/sys-info.rst
44 ↗	(On Diff #3727)	I wonder how to explain this in the cli... Also, that might be a possible improvment to add that check... I don't know how to be 'dry' though. That check is already done in the settings.production... nevermind, i'll update the diff.

ardumont marked an inline comment as done.Feb 24 2019, 10:05 AM

ardumont added inline comments.

docs/sys-info.rst
44 ↗	(On Diff #3727)	In D1184#25162, i raise a more explicit message for the missing tested case (env variable not set).

Build is green
See https://jenkins.softwareheritage.org/job/DDEP/job/tox/87/ for more details.

Harbormaster completed remote builds in B4419: Diff 3728.Feb 24 2019, 10:05 AM

swh.manage: Migrate manage to swh/deposit subfolder

Unrelated but since i fixed issues already. I noticed that the
manage.py module in questions ends up in the wrong root tree once
installed.

Build is green
See https://jenkins.softwareheritage.org/job/DDEP/job/tox/88/ for more details.

Harbormaster completed remote builds in B4420: Diff 3729.Feb 24 2019, 7:14 PM

I'm globally OK with this, but please add support for a --config-file cmd line option in parallel with the environment variable support, as other swh-xxx commands do.

And we might want to refactor the swh-deposit-client tool as a swh-deposit client subcommand instead, but that will be another diff.

docs/dev-info.rst
121 ↗	(On Diff #3729)	described
122 ↗	(On Diff #3729)	maybe add a line describing what are these private configs for?
docs/sys-info.rst
44–45 ↗	(On Diff #3729)	For a command line example like this, I'd rather use something like: swh-deposit -C /etc/softwareheritage/deposit/server.yml --platform production \ [...] rather than having to define the env var... Note: I've not verified whether the swh-deposit command support a -C option, but you get the point.
setup.py
56 ↗	(On Diff #3727)	@douardda In any case, it might be worth considering to merge that in the main cli? yes this is something we might want at some point, but it's not that easy to do properly, we might need lazy loading mechanisms to prevent the tool to be too slugish for example...
56 ↗	(On Diff #3727)	context: Deposit is split in 3 debian packages (python3-swh.deposit, python3-swh.deposit.client, python3-swh.deposit.loader) so this will be installed broken for some of those (i think). I guess both python3-swh.deposit.client and python3-swh.deposit.loader depends on python3-swh.deposit, so why not have a single swh-deposit entry point that comes with this later? Then, a simple try: import swh.deposit.client.cli except ImportError: pass in `swh.deposit.cli` should do the trick, I guess. Sure, the `swh.deposit.client.cli` should be modified to use the `swh.deposit.cli.cli` main group instead of redefinig one. Same applies to `swh.deposit.loader`.

This revision now requires changes to proceed.Mar 8 2019, 11:35 AM

I'm globally OK with this, but please add support for a --config-file cmd line option in parallel with the environment variable support, as other swh-xxx commands do.

I planned on doing that in another diff.
I'm doing stuff incrementally. That's easier to review.

Here the goal was to align with other modules to have a swh.<module>.cli as before the script was there but dedicated to one action (that's what the title tried to convey).
Now, with this it's aligned.

I added the collection support which should have been in another diff already ;)

setup.py
56 ↗	(On Diff #3727)	I guess both python3-swh.deposit.client and python3-swh.deposit.loader depends on python3-swh.deposit, No they don't, description is: python3-swh.deposit: public exposed sword api, private api/rpc server (for loaders) python3-swh.deposit.loader: loader, they use a client to discuss with the private api of the server python3-swh.deposit.client: http client to discuss with the public api sword of the server Dependency is only: python3-swh.deposit.loader -depends-on-> python3-swh.deposit.client

Ok to go incrementally like that, but ensure a task/diff making this cli more consistent with other swh commands exists.

This revision is now accepted and ready to land.Mar 12 2019, 2:30 PM

In D1184#26257, @douardda wrote:

Ok to go incrementally like that, but ensure a task/diff making this cli more consistent with other swh commands exists.

T1580!

Thanks for the review.

Closed by commit rDDEPce40f37345d4: swh.deposit.cli: Transform create_user into a main cli (authored by ardumont). · Explain WhyMar 12 2019, 4:03 PM

This revision was automatically updated to reflect the committed changes.