Replace conflicting repository-service-tuf dep #16098
Conversation
There was a problem hiding this comment.
Code looks fine, but an organization question: are these functions going to be used outside of the cli.tuf module? If not IMO we could inline them there.
There was a problem hiding this comment.
Yes, I plan to use the same "RSTUF API library" to "add targets", i.e. tell rstuf to update tuf metadata. See #15815
| return resp_data["task_id"] | ||
|
|
||
|
|
||
| def wait_for_success(server: str, task_id: str): |
There was a problem hiding this comment.
@woodruffw, in case you are familiar with warehouse's async task machinery, do you think it's worth to make this a task, which calls retry() until "SUCCESS" or max retry instead of looping?
I thought it might be more elegant, but I didn't get it to work (I'm pretty unfamiliar with Celery).
There was a problem hiding this comment.
Yeah, this could probably be done with a custom celery.Task, although it's been a few years since I've mucked around in Celery 🙂
IMO this is fine as is, although longer term either a Celery task integration or a synchronous wrapper would be ideal.
There was a problem hiding this comment.
I guess I'll leave it as is. My plan for #15815 is to already make the calling function (update_metadata) an async task, which blocks on wait_for_success. So making the latter async as well, maybe isn't helpful. But I'll let the Warehouse maintainers comment.
Thanks for taking a look!
|
Just pushed some tests. Coverage passes now. Please take another look. |
There was a problem hiding this comment.
LGTM! CC @facutuesca for a review as well, to confirm that this handles the dep issue sufficiently 🙂
|
(This will need a rebase/fast-forward onto |
Yes, this fixes it. Thanks @lukpueh !! |
Previously, `repository-service-tuf` (i.e. the RSTUF cli) was used to bootstrap an RSTUF repo for development. This PR re-implements the relevant parts of the cli locally in Warehouse and removes the `repository-service-tuf` dependency, which conflicts with other dependencies. Change details - Add lightweight RSTUF API client library (can be re-used for pypi#15815) - Add local `warehouse tuf bootstrap` cli subcommand, to wraps lib calls - Invoke local cli via `make inittuf` - Remove dependency supersedes pypi#15958 (cc @facutuesca @woodruffw) Signed-off-by: Lukas Puehringer <[email protected]>
Other than the regular click File, the LazyFile also has the "name" attribute, when passing stdin via "-". We print the name on success. Signed-off-by: Lukas Puehringer <[email protected]>
Signed-off-by: Lukas Puehringer <[email protected]>
Signed-off-by: Lukas Puehringer <[email protected]>
lukpueh
force-pushed
the
tmp-rm-rstuf-cli
branch
from
0acb331 to
36b861c
Compare
| import click | ||
|
|
||
| from warehouse.cli import warehouse | ||
| from warehouse.tuf import post_bootstrap, wait_for_success |
There was a problem hiding this comment.
In other CLI modules, we move the imports to the command that uses them, often with this comment, so that the CLI startup time is pretty lean.
# Imported here because we don't want to trigger an import from anything
# but warehouse.cli at the module scope.
Looking at warehouse.tuf, it doesn't currently import anything that I'm particularly worried will be slow, so happy to defer that until there's more, but wanted to highlight it.
Previously,
repository-service-tuf(i.e. the RSTUF cli) was used to bootstrap an RSTUF repo for development. This PR re-implements the relevant parts of the cli locally in Warehouse and removes therepository-service-tufdependency, which conflicts with other dependencies.Change details
warehouse tuf bootstrapcli subcommand, to wraps lib callsmake inittufsupersedes #15958 (cc @facutuesca @woodruffw)