Details
Diff Detail
- Repository
- rDDATASET Datasets
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Most of my comments are minor/nice to have, although I'd like to be able to pass queries directly on the CLI.
swh/dataset/athena.py | ||
---|---|---|
1 ↗ | (On Diff #19691) | this lacks the usual copyright/license blurb |
1 ↗ | (On Diff #19691) | also, a short docstring stating this module implements the "athena" subcommands for the CLI would be nice to have |
124 ↗ | (On Diff #19691) | KiB, MiB, GiB, etc. ;-) |
swh/dataset/cli.py | ||
130 ↗ | (On Diff #19691) | maybe "manage and query" |
171 ↗ | (On Diff #19691) | So the query must be given in a file? That seems cumbersome. How about expecting it as a string argument, either as an option, but even as the only way to pass a query; after all one can always use shell expansion $(cat ...) if the query is too long for the command line. |
swh/dataset/relational.py | ||
1 | a comment line before the formatting pragma stating what this module contains would be nice to have |
Thanks for the review!
swh/dataset/cli.py | ||
---|---|---|
171 ↗ | (On Diff #19691) | I'm mimicking how most database command lines work. You can also write <<<"query" if you want to inline it (or echo "query" | swh athena query), and heredocs are more natural this way. |