Draft: Feature: Add support for generating json from bst show command#2030
Draft: Feature: Add support for generating json from bst show command#2030kevinschoon wants to merge 1 commit intoapache:masterfrom
Conversation
kevinschoon
requested review from
BenjaminSchubert,
abderrahim,
cs-shadow,
gtristan and
juergbi
as code owners
kevinschoon
force-pushed
the
ks/add-json-support-to-bst-show
branch
from
69494ab to
df01c09
Compare
This adds a new flag called --json to the `bst show` command which when specified will make bst output a machine readable JSON dump of the dependency state within the current buildstream project. Having JSON support makes it easier to write tools that need to inspect the configuration of a buildstream enabled project.
kevinschoon
force-pushed
the
ks/add-json-support-to-bst-show
branch
from
df01c09 to
524c1a4
Compare
abderrahim
left a comment
abderrahim
left a comment
There was a problem hiding this comment.
Overall, I like this. It's more easily parsed, and doesn't suffer from the pitfalls of trying to format yaml using the current --format option.
I think adding yaml output with the same semantics would be good, especially as buildstream is already yaml-centric.
The implementation itself uses names that are already taken, but that shouldn't be hard to fix.
| @click.option( | ||
| "--except", "except_", multiple=True, type=click.Path(readable=False), help="Except certain dependencies" | ||
| ) | ||
| @click.option("--json", "as_json", is_flag=True, help="Output the information as machine readable JSON") |
There was a problem hiding this comment.
It would be nice to have a similar option that outputs YAML, as that's what buildstream uses.
| ], | ||
| ), | ||
| help="The dependencies to show", | ||
| help="Types of Elements to Show", |
There was a problem hiding this comment.
Please keep the old description. "Types of elements" doesn't make sense here.
| show_default=True, | ||
| multiple=True, | ||
| type=FastEnumType( | ||
| _ElementKind, |
There was a problem hiding this comment.
"Element Kind" is definitely not how you should name this. The element kind is already a concept in buildstream, and it's not this.
I don't know what would be the best way to name this option though. info? keys?
Let's wait for other maintainer's opinions, they might have a better idea.
| raise AppError("--format and --json are mutually exclusive") | ||
|
|
||
| if format_ and kind: | ||
| raise AppError("--format does not support --kind") |
There was a problem hiding this comment.
Not sure I like this error message, as I see --format and --kind (in your current implementation) to be essentially the same thing except --format is used in the default mode and --kind is used in the json mode.
Maybe we could change it such that --format could take the special value json, and that would unlock the (replacement for the) --kind option.
Let's wait for other opinions.
|
Oh, I forgot. This also needs tests. You can probably just add a test or two to |
kevinschoon
changed the title
|
Closing this in favor of #2035 |
2 participants
This adds a new flag called --json to the
bst showcommand which when specified will make bst output a machine readable JSON dump of the dependency state within the current buildstream project. Having JSON support makes it easier to write tools that need to inspect the configuration of a buildstream enabled project.