Skip to content

add monitor subcommands that work from file#322

Merged
yannmh merged 3 commits intoDataDog:masterfrom
Hefeweizen:update_files
Mar 20, 2019
Merged

add monitor subcommands that work from file#322
yannmh merged 3 commits intoDataDog:masterfrom
Hefeweizen:update_files

Conversation

@Hefeweizen
Copy link
Copy Markdown
Contributor

These subcommands read a filename from the command line, and then use
specified file for all values. These features allow a workflow of dog monitor show <id> > monitor_id.json && dog monitor fupdate monitor_id.json. This workflow allows version control of all monitors,
as well as repo-driven updates.

These subcommands read a filename from the command line, and then use
specified file for all values.  These features allow a workflow of `dog
monitor show <id> > monitor_id.json && dog monitor fupdate
monitor_id.json`.  This workflow allows version control of all monitors,
as well as repo-driven updates.
@yannmh
Copy link
Copy Markdown

yannmh commented Mar 20, 2019

This is very helpful. Thank you @Hefeweizen

@yannmh yannmh merged commit 4ee67b9 into DataDog:master Mar 20, 2019
@Hefeweizen
Copy link
Copy Markdown
Contributor Author

@yannmh Thanks; I appreciate the review.

I did mvp for my needs and only got this working with monitors. I think it'd be nice if this capability (load from file) were available for all object types (e.g. dashboards, &c). Should I piecemeal this in on each subcommand, or is there something architectural that could be done?

@yannmh
Copy link
Copy Markdown

yannmh commented Mar 21, 2019

I agree this would also be great for other resources.
Adding a fpost and fupdate to all of them doesn't feel right ... instead I'd like to explore the possibility of re-using the existing "post" and "update" subcommands and maybe some extra parameter to specific that the we intend to pass content as json, e.g. something like

cat my_monitor.json | dog monitor update --json

@Hefeweizen
Copy link
Copy Markdown
Contributor Author

@yannmh I chose fupdate/fpost because I just wanted traction on the idea and to get something started without having to build too much argument handling into the existing _post. For better integration, it seems something like this may work:

    def _post(cls, args):
       if args.json:
          _post_stdin()
       else:
         _post_from_args()

    def _post_from_args(cls, args):
        """
        previously, just `_post()`
        """
        api._timeout = args.timeout
        format = args.format
        options = None
        ....

    def _post_stdin(cls, args):
       monitor = json.load(sys.stdin)
       ....

This provides the capability locally, but we'd have to make the changes across every subcommand (monitors, dashboards, &c). Is there anything we can do structurally to provide the capability across all of them in one swoop?

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