Add revoke method to Screenboard
#46
Conversation
|
Looks great. Thanks @allegrem |
|
I will add some tests for this PR as soon as possible |
- This method allow to revoke the public access of a shared screenboard
allegrem
force-pushed
the
allegrem/revoke-screenboard
branch
from
5b13c6f to
ea95011
Compare
| @@ -553,6 +554,17 @@ def _compare_screenboard(board1, board2): | |||
|
|
|||
| share_res = dog.Screenboard.share(get_res['id']) | |||
| assert share_res['board_id'] == get_res['id'] | |||
| public_url = share_res['public_url'].replace('8080', '5000') | |||
There was a problem hiding this comment.
The replace('8080', '5000') is a bit annoying, but I can't find a way to get rid of it in local env. Any idea @yannmh ? I guess it would work when testing against the staging env.
There was a problem hiding this comment.
I am usually testing against staging or a prod demo account, so I never experienced this inconvenience before.
Still, I don't think we should commit this extra line: it's too much conditioned by your environment thus it could easily break in a different scenario (i.e. testing from your VM or not).
An other option would be to make this more flexible using an environment variable like we do to set the Datadog URL ? What do you think ?
allegrem
force-pushed
the
allegrem/revoke-screenboard
branch
from
ea95011 to
1abe36b
Compare
| out = json.loads(out) | ||
| assert out['board_id'] == screenboard['id'] | ||
| # Verify it's actually shared | ||
| public_url = out['public_url'].replace('8080', '5000') |
- Add tests for 'screenboard revoke' command - Also add tests for 'screenboard share' command
allegrem
force-pushed
the
allegrem/revoke-screenboard
branch
from
1abe36b to
77b3556
Compare
| @@ -9,6 +9,7 @@ | |||
| import time | |||
| import tempfile | |||
| import unittest | |||
| import urllib2 | |||
There was a problem hiding this comment.
Let's use requests instead. Easier, shorter and compatible Python 2.6—3.4 (not the case for urllib2) !
|
Thanks for the new feature @allegrem 💥 |
- Too specific to my dev environment
|
I fixed everything you suggested @yannmh . Ready to merge? |
|
👍 ! |
Add `revoke` method to Screenboard
cc @yannmh
Related to DataDog/documentation#395