-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
check that share key is enabled #726
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
Conversation
plotly/plotly/plotly.py
Outdated
@@ -1308,7 +1308,14 @@ def add_share_key_to_url(plot_url): | |||
body = {'share_key_enabled': True, 'world_readable': False} | |||
response = v2.files.update(fid, body) | |||
|
|||
return plot_url + '?share_key=' + response.json()['share_key'] | |||
if not v2.files.retrieve(fid).json()['share_key_enabled']: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add a comment here about why we're trying multiple times
plotly/plotly/plotly.py
Outdated
return plot_url + '?share_key=' + response.json()['share_key'] | ||
if not v2.files.retrieve(fid).json()['share_key_enabled']: | ||
attempt += 1 | ||
if attempt == 10: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be OK with bumping this up to 50 even
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 bumped to 50
plotly/plotly/plotly.py
Outdated
if not v2.files.retrieve(fid).json()['share_key_enabled']: | ||
attempt += 1 | ||
if attempt == 10: | ||
raise Exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're trying to wrap everything in plotly exceptions so that users can catch them. So just: raise exceptions.PlotlyException
(or something)
💃 once you consider my comments |
No description provided.