DM-25196 make query-collections and query-dataset-types butler commands #304
Conversation
|
I've asked in Slack if |
| def makeCollectionType(self, context, param, value): | ||
| if value is None: | ||
| return value | ||
| value = value.upper() |
There was a problem hiding this comment.
Click doesn't normalize case on choice options if you say case_sensitive=False ? Does click do minimum matching so that "chain" would work as well as "chained"?
|
|
||
| Returns | ||
| ------- | ||
| collections : [`str`, [`str`]] |
There was a problem hiding this comment.
I'm trying to parse this return type indicator. Is that how we show that a dict is returned?
|
|
||
|
|
||
| def queryDatasetTypes(repo): | ||
| """Get the dataset types in a repository. |
| """ | ||
| butler = Butler(repo) | ||
| datasetTypes = butler.registry.queryDatasetTypes() | ||
| return {'dataset-types': [datasetType.name for datasetType in datasetTypes]} |
There was a problem hiding this comment.
I think I prefer "datasetTypes" as the key.
tests/test_cliCmdQueryCollections.py
Outdated
| expected = {"collections": ["ingest/run", "ingest"]} | ||
| runner = click.testing.CliRunner() | ||
| with runner.isolated_filesystem(): | ||
| _ = Butler.makeRepo("here") |
There was a problem hiding this comment.
Why are you assigning anyything at all to the return value?
The more common idiom though is for the return value from Butler.makeRepo and pass it to the first argument of the Butler constructor in the next line.
tests/test_cliCmdQueryCollections.py
Outdated
| runner = click.testing.CliRunner() | ||
| with runner.isolated_filesystem(): | ||
| _ = Butler.makeRepo("here") | ||
| _ = Butler("here", run=run, tags=[tag], collections=[tag]) |
There was a problem hiding this comment.
Can you add a comment here to say that the purpose of this call is to create some collections?
tests/test_cliCmdQueryCollections.py
Outdated
| def testGetCollections(self): | ||
| run = "ingest/run" | ||
| tag = "ingest" | ||
| expected = {"collections": ["ingest/run", "ingest"]} |
There was a problem hiding this comment.
Don't repeat yourself here. use [run, tag] here
| runner = click.testing.CliRunner() | ||
| with runner.isolated_filesystem(): | ||
| Butler.makeRepo("here") | ||
| butler = Butler("here", writeable=True) |
There was a problem hiding this comment.
Again I have a slight preference for the return value of makeRepo being used here.
No description provided.