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

Add v2 of the dashboard_list_item API #262

Merged
merged 3 commits into from
Aug 22, 2019

Conversation

nmuesch
Copy link
Collaborator

@nmuesch nmuesch commented Aug 19, 2019

Adds the v2 version of the dashboard list item API - https://docs.datadoghq.com/api/?lang=python#get-items-of-a-dashboard-list

Makes new methods for each CRUD operation (same names as the V1 version but appended with V2)

Only the individual items API has a V2, so this doesn't duplicate the V1 dashboard_list API. Mostly the same, but the Item struct ID field is now all capitalized, and returns a string instead of an integer.

Copy link
Contributor

@jbenais jbenais left a comment

Choose a reason for hiding this comment

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

LGTM! 👍Could you add tests for those methods?

@nmuesch
Copy link
Collaborator Author

nmuesch commented Aug 20, 2019

@jbenais Thanks for the review. Added a set of tests (its pretty much identical to the tests for v1, but updated to use the new structs/methods) Let me know what you think.

I also don't have a strong preference but found myself referring to these as DashboardListV2Items vs having the V2 after the word items. Let me know if you think one is preferential to the other. (The API methods are consistent)

Copy link
Contributor

@jbenais jbenais left a comment

Choose a reason for hiding this comment

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

💯

@bkabrda
Copy link
Collaborator

bkabrda commented Aug 22, 2019

LGTM, merging 👍 Thanks for doing this!

@bkabrda bkabrda merged commit e9c4097 into zorkian:master Aug 22, 2019
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.

4 participants