Skip to content

Conversation

@kenlavoie
Copy link

No description provided.

Ken Lavoie added 2 commits January 14, 2019 12:00
We currently do not have a way to download a single dashboard. A user has to pull the dashboards and perform the data manipulation 

This PR allows the end user to enter a dashboard, and download a single dashboard 

dashboard_number does need a better way to input the value
removed additional value not needed
@figarocorso
Copy link
Contributor

Hi @kenlavoie,

As this package is used as an API wrapper we only expect returning API outcome. Thus all the code related with writing the file to disk shouldn't be here. It should be at our users scripts (as you can find at this example: https://github.com/draios/python-sdc-client/blob/master/examples/dashboard_save_load.py#L37 )

If we remove that from your PR, we have some code looking for a given dashboard. But we already have that feature using the find_dashboard_by method to achieve that (https://github.com/draios/python-sdc-client/blob/master/examples/dashboard.py#L61 )

Thank you for the interest and effort, but I think we cannot merge this PR as it is.

Ps: Maybe I am missing something, thus please, post your reply if wanted.
Ps2: If opening a new PR or changing this one, please try to write your code similarly to the one already at the repo, so we keep the style under control (comments, naming, ...)

@davideschiera
Copy link
Contributor

@kenlavoie If I understand correctly, the functionality allows you to find a dashboard and store the configuration in a file on disk. If this is correct, together with the work to support the latest dashboards API we are adding backup/restore functionalities in the library (mainly to properly handle versioning). This seems to overlap with the functionality you are proposing.

Also, I agree with @figarocorso review.

Please let us know what you think. I plan to close the PR by the time we release support for latest dashboards API, unless I hear otherwise. We can always reevaluate things later anyway ;-)

Thanks!

@kenlavoie
Copy link
Author

kenlavoie commented Mar 29, 2019 via email

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.

3 participants