Download and cache plot schema (or always download if no file permissions). #284
Conversation
Conflicts: plotly/tools.py
* no need to reset the graph reference file * send a hash along with the request to check for an update * if the request fails *and* there’s no local reference —> error.
This is super annoying and I wasn’t able to see a simple way of getting `six` to help us out… This is ugly but it should work.
You use `make update_default_schema` from the top-level repo file.
This means we *never* throw an error. Order of priority: 1. we successfully check with the plotly api 2. check fails, but there’s a local backup 3. check fails and no local backup. we resort to default file
|
Looking good. About
How often should we update the default schema? |
|
I'm not opposed to bumping the minor version number as often as plot schema changes - a robot could make PRs. That's a nice fallback in case downloading the plot schema automatically doesn't work for some reason for users, they could always just |
|
No real opinion on the upgrade frequency. A bot could load the default json dict, hash it and check if it needs updating daily, pushing to master and then uploading to PyPI on changes. I'll open up an issue to discuss on the side, #296. |
| sha1 = hashlib.sha1(six.b(str(graph_reference))).hexdigest() | ||
|
|
||
| graph_reference_url = '{}{}?sha1={}'.format(plotly_api_domain, | ||
| GRAPH_REFERENCE_PATH, sha1) |
There was a problem hiding this comment.
Yikes, I didn't think about this. A few things to consider:
- Is this going to break for the plotly enterprises out there that don't have
v2endpoints? - If we keep seperate versions of plot-schema for whichever plotly.js is being used, we'll need another version for plotly offline and whatever version of plot-schema it uses.
There was a problem hiding this comment.
It won't break enterprise solutions (more than they're already broken). They have a default schema to fall back on. This also has the deprecated stuff worked in, so it'll be allowed. A funny situation would be if the plotlyjs they are using hasn't deprecated a key and we throw a warning suggesting they use a key that actually doesn't exist yet...
There was a problem hiding this comment.
@chriddyp you're gonna need to refresh me on the plotly offline here 🐱. Do we have a plot schema for the plotly-offline that we can get at somewhere?
There was a problem hiding this comment.
Presumably, when plotly offline downloads (the latest) plotly.js, it can also download that version's plot-schema. This schema won't need updating, since it is locked in with the local plotly.js.
Conflicts: plotly/tools.py
|
Anything blocking this beyond the timeout discussion? I think it will make the update-graph-reference pr a little cleaner if I can get this merged into master and then master merged into that branch. |
|
no objections here.. |
|
boom! |
|
@cldougl , yep, the merge conflict is taken care of, thanks for the reminder! just let me know if/when it can 💃 :) @chriddyp also looking for confirmation on the |
|
💃 |
|
K, timeout added and default schema updated. mergin' |
Download and cache plot schema (or always download if no file permissions).
This PR handles getting the new plot schema downloaded/cached/backed-up within the old framework. There's no change to code usage here, just JSON handling.
Note that if enterprise doesn't have the new endpoint defined, the default one can be used. This doesn't guarantee that the plotlyjs running on the enterprise machine is in sync with the plotlyjs used to create the default though! In particular, the
autoticktotickmodechange on axes inside the mpl renderer may be problematic. Hopefully this will get knocked to a warning though (_deprecated) and will eventually stabilize once enterprise instances get updated versions.Todo:
for no file permissions and loaded graph referencethis will get checked on circle