Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement mapped collection create #898

Closed
wants to merge 5 commits into from

Conversation

sirosen
Copy link
Member

@sirosen sirosen commented Nov 28, 2023

This is a rebase of the prior work which I had on this, with some minimal cleanup to get things into a reasonable state. I'm opening it as a draft to improve visibility. It conflicts with the guest collection creation command and will need a rebase after that merges (so please ignore the conflicts).

I see a few things which need additional attention in here, and I especially note that there were no tests when I dusted this off. I think I had done some measure of manual testing which at this point is not very relevant.

Some areas of interest:

  • This shares parameters with collection update, and it will share some with guest creation as well -- we should move those to common helpers where possible.
  • Handling of the policies sub-object is tricky. The current implementation has a small collection of mutually exclusive options which can be used. (These could be declared as mutex, but I believe mutex checking of multiple=True options requires some care.) I would like it better if we could declare these usages separately somehow, but I think the current solution is likely to be a good pragmatic solution for the near-to-medium term.
  • The commit history notes an interesting rationale for naming the option --storage-gateway rather than --storage-gateway-id. The expectation baked into this is that a typical endpoint will have only one or two gateways attached, and that typically when there are two (or more), there will only be one of each type. It is therefore a planned enhancement to support --storage-gateway posix as a method for selecting the only POSIX gateway on the Endpoint. I think this branch of work should be updated to either implement that logic OR switch to --storage-gateway-id. I'm not yet sure which.

This is an almost direct port of the gcs-cli command with no
significant changes. No tests are included yet.
`storage_gateway_id` and `base_path` are now specified via options
rather than positionals. These have two advantages:
- order independent (too many positional arguments becomes confusing)
- we can specify defaults

Because we make a strong habit of avoiding required options, it is
desirable to have some default behaviors.
`--base-path=/` is simple, but `--storage-gateway` (note that it does
not end in `-id`) is more complex.

After discussion, we're going to have it default to "the only gateway
on the endpoint", and leave any more sophisticated behaviors as future
work. Future possibilities include

    # selecting by type
    --storage-gateway=posix

    # selecting by name
    --storage-gateway=mygateway

    # selecting by any of the above with prefix mapping
    --storage-gateway="type:posix"
    --storage-gateway="name:mygateway"
Adjust option declarations, ensure they pass mypy and the
click-type-test integration, and update the phrasing in many small
ways to match the current expectations and usages of the CLI.
This primarily consists of a new set of tests which exercise the
command exactly as written. One thing this revealed right away was
that the CLI output from mapped collection creation was missing most
of the useful fields to inspect. To remedy this, the command was
updated with the "standard_collection_fields".
@sirosen sirosen force-pushed the implement-collection-create branch from 1d91c7c to a891dd7 Compare December 13, 2023 17:40
@sirosen
Copy link
Member Author

sirosen commented Dec 13, 2023

Superseded by #919

@sirosen sirosen closed this Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant