Create a custom artifact-ref click parameter type
- Dominant language
- Python
- Stars
- 140
- Forks
- 45
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 6
Description
[See original issue on GitLab](https://gitlab.com/BuildStream/buildstream/-/issues/1091)
In GitLab by [[Gitlab user @tlater]](https://gitlab.com/tlater) on Jul 26, 2019, 12:13
As part of #773, we will need to update the semantics of how `element` types are treated in the CLI. Currently, they are all `click.Path`, but it's not guaranteed that `click.Path` validation will always allow any artifact ref, so technically there's a chance that BuildStream rejects certain artifact refs on some non-Unix OSs (although I suspect that BuildStream would break much more horribly in such cases anyway). If we change the type to something like `str`, however, we lose completion.
We should create a custom click type to handle artifact refs. This would also allow implementing auto completion for artifact refs, which is something we want to do for #773 anyway.
To resolve this issue, we'd need to:
- [ ] Create an `Artifact` `click.ParamType` and use it throughout `cli.py` where it makes sense, without regressing file name completion.
- [ ] Add artifact ref completion for this type.
The first part should be relatively doable, and can land without the second part. So it may be worth trying if you're a newcomer :)
Relevant discussion from !1497:
- [ ] [[Gitlab user @tlater]](https://gitlab.com/tlater) started a [discussion](https://gitlab.com/BuildStream/buildstream/merge_requests/1497#note_196340365): (+2 comments)
> Sorry, I went and thought about this some more.
>
> We should be doing this:
>
> ```suggestion:-0+0
> type=str)
> ```
>
> Why? Well, click verifies that the `Path` is a correct file path. We don't want that, since an artifact ref may be an invalid file path (or well, in theory we could change the artifact ref format to allow invalid file paths, or we may run on an OS where they are).
>
> Ideally, we should create a [custom type](https://click.palletsprojects.com/en/7.x/parameters/#implementing-custom-types) that is an artifact ref that could be either a file name or a ref, but I'm ok with just making this a `str`. It should *not* be a `Path` anymore, though.
Note that this is not possible since we still rely on `click.Path` for completion.
Contributor guide
Research direction
Start in cli.py, where element parameters currently use click.Path, and review the custom click.ParamType guidance linked in the issue. Determine how artifact references and file names should be distinguished without regressing file-name completion. Done means an Artifact parameter type is used where appropriate and artifact-reference completion is addressed, or the first part is delivered independently as permitted by the issue.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- cli
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100