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

Adding --out flag #748

Merged
merged 3 commits into from
Feb 14, 2020
Merged

Conversation

jonathan-buttner
Copy link
Contributor

This PR adds the --out switch to specify where the generated files and docs are to be written. If the directory does not exist it will be created.



def argument_parser():
parser = argparse.ArgumentParser()
parser.add_argument('--intermediate-only', action='store_true',
help='generate intermediary files only')
parser.add_argument('--include', action='store',
parser.add_argument('--include', action='append',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the change for append here, if you pass in multiple --include flags it will only use the last one. This causes the values to be added to an array.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think we should align how we do multiple values, between --subset and --include.

  • include: --include path1 --include path2
  • subset: --subset path1 path2

I have a preference for how we do --subset because it shows up clearly in the help:

usage: generator.py [-h] [--intermediate-only] [--include INCLUDE]
                    [--subset SUBSET [SUBSET ...]] [--out OUT]

optional arguments:
  -h, --help            show this help message and exit
  --intermediate-only   generate intermediary files only
  --include INCLUDE     include user specified directory of custom field
                        definitions
  --subset SUBSET [SUBSET ...]
                        render a subset of the schema
  --out OUT             directory to store the generated files

And I suspect --subset will play better with users using globs without quoting, and their shell expanding to a list of file names.

WDYT? Are there downsides nargs='+'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good point, didn't think of that. I don't think there should be any issue with switching it over to nargs. I'll update it.

@jonathan-buttner jonathan-buttner changed the title Adding out parameter Adding --out flag Feb 13, 2020
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this, thanks @jonathan-buttner!

Just need to align on how we do multiple args for include vs subset, see comment below.

Then we can merge 🎉



def argument_parser():
parser = argparse.ArgumentParser()
parser.add_argument('--intermediate-only', action='store_true',
help='generate intermediary files only')
parser.add_argument('--include', action='store',
parser.add_argument('--include', action='append',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think we should align how we do multiple values, between --subset and --include.

  • include: --include path1 --include path2
  • subset: --subset path1 path2

I have a preference for how we do --subset because it shows up clearly in the help:

usage: generator.py [-h] [--intermediate-only] [--include INCLUDE]
                    [--subset SUBSET [SUBSET ...]] [--out OUT]

optional arguments:
  -h, --help            show this help message and exit
  --intermediate-only   generate intermediary files only
  --include INCLUDE     include user specified directory of custom field
                        definitions
  --subset SUBSET [SUBSET ...]
                        render a subset of the schema
  --out OUT             directory to store the generated files

And I suspect --subset will play better with users using globs without quoting, and their shell expanding to a list of file names.

WDYT? Are there downsides nargs='+'?

@jonathan-buttner
Copy link
Contributor Author

Example run:

python scripts/generator.py --include ../test1 ../test2 --out ../test_out
Running generator. ECS version 1.5.0-dev
Loading default schemas
Loading user defined schemas: ['../test1/agent.yml', '../test2/http.yml']

@webmat
Copy link
Contributor

webmat commented Feb 14, 2020

Awesome, thank!

  --include INCLUDE [INCLUDE ...]
                        include user specified directory of custom field
                        definitions
  --subset SUBSET [SUBSET ...]
                        render a subset of the schema

❤️

@webmat webmat merged commit 4c91c2e into elastic:master Feb 14, 2020
@jonathan-buttner jonathan-buttner deleted the feature/minor-fixes branch February 14, 2020 15:45
dcode pushed a commit to dcode/ecs that referenced this pull request Apr 15, 2020
Allows for the artifacts to be output to a different directory.

This is useful for users who leverage the ECS tooling to manage their Elasticsearch templates.
@ebeahan ebeahan mentioned this pull request Jul 9, 2020
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.

2 participants