') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Hot reload by T4rk1n · Pull Request #362 · plotly/dash · GitHub
Skip to content

Hot reload - #362

Merged
T4rk1n merged 29 commits into
masterfrom
hot-reload
Nov 14, 2018
Merged

Hot reload#362
T4rk1n merged 29 commits into
masterfrom
hot-reload

Conversation

@T4rk1n

@T4rk1nT4rk1n commented Aug 31, 2018

Copy link
Copy Markdown
Contributor

Available with the following versions:

  • dash==0.29.0rc4
  • dash-renderer==0.15.0rc2

Reload the browser when files are changed.

  • Use the flask debug reloader to reset the hash on python file change.
  • Watch the assets files and change the hash on assets modified.
  • Add a route to serve the reload hash.

Dev tools params (in app.run_server or app.enable_dev_tools

  • dev_tools_hot_reload, bool set to true to enable hot reload (default=False).
  • dev_tools_hot_reload_interval, int, interval at which the renderer will request the reload hash (default=3000).
  • dev_tools_hot_reload_watch_interval, float, The delay between each walk of the assets folder to detect file changes.
  • dev_tools_silence_routes_logging, disable the route logging.

Hot reload is activated by default when debug=True !!

Need plotly/dash-renderer#73.

Resolves#66.
Fix#341.

@T4rk1nT4rk1n mentioned this pull request Aug 31, 2018
@T4rk1n
T4rk1nforce-pushed the hot-reload branch 2 times, most recently from 9a48f6c to 8955a92CompareSeptember 5, 2018 17:18
@T4rk1nT4rk1n changed the title [WIP] Hot reloadHot reloadSep 5, 2018
@T4rk1n
T4rk1n requested review from ned2 and rmarren1September 11, 2018 20:00
@T4rk1n

Copy link
Copy Markdown
ContributorAuthor

@plotly/dash Ready for reviews!

Comment threaddash/_watch.py
if pattern and not pattern.search(f):
continue
path = os.path.join(current, f)
info = os.stat(path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

os.path.getmtime might be cleaner here. I thought there was platform dependence issue at first (since otherwise why would they even implement os.path.getmtime if you can just os.stat(path).st_mtime, but it turns out thats exactly what os.path.getmtime does.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yea, some methods in that module seems to be wrapper around os.stat. I'm used to os.stat so I just do that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍, either way is good, just seemed a bit strange, seems to break "There should be one-- and preferably only one --obvious way to do it".

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I think the os module predates the PEP's.

Comment threaddash/_watch.py

def watch(folders, on_change, pattern=None, sleep_time=0.1):
pattern = re.compile(pattern) if pattern else None
watched = collections.defaultdict(lambda: -1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Neat, didn't know you could do this.

Comment threaddash/dash.py Outdated
self._watch_thread = threading.Thread(
target=lambda: _watch.watch([self._assets_folder],
self._on_assets_change,
sleep_time=0.5))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't sleep_time be hot_reload_interval / 1000? I see that hot_reload_interval is passed to the front-end via config, but I'm not sure if it is intended to be the same on the back-end. If not (e.g. back-end should have a fixed constant refresh interval regardless of hot_reload_interval value) I think you should make this a constant set in the _watch file.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The sleep time is the poll time for the assets watch thread and hot_reload_interval is for the client requests interval. I don't think the user will need to change the sleep time so it can be a constant.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to make these two times the same, or at least proportional. With this there is a .5 second lower bound on the hot reload interval, and with large intervals (e.g. 10 seconds) you would be making a lot of calls to watch that aren't really necessary, if a user has a huge app that takes a non-trivial amount of computation to traverse the asset tree, this might affect performance.

I think it makes sense to make sleep_time = (hot_reload_interval / 1000) / d where d is some constant. Then in the worst case after making a change, a user would need to wait hot_reload_interval + (hot_reload_interval / d) time for the page to change. d = 2 or d = 4 seems reasonable.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't think it matter much, it just the timer for when to run the check in the back end and should not be linked in any way to the request timer that can be changed. It should be a constant that work goods in the conditions. We don't want run that check too often because of the GIL and it should still be low so you can get the reload a short while after making changes. So I think a check every half a second is good with this in mind.

In my asyncio based implementation it was a really low number like yield from asyncio.sleep(0.000001) and it didn't make a difference in the performance of the app.

Comment threaddash/dash.py Outdated
self._watch_thread.start()

if not self.server.debug:
self.server.debug = True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this would get over-ridden when somebody calls app.run_server(debug=False). Should that happen? I think in run_server you can just say debug = debug or self.hot_reload

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I think I will remove that from here and add it in the enable method from #369.

Comment threaddash/dash.py Outdated
def serve_reload_hash(self):
return flask.jsonify({
'reloadHash': self._reload_hash,
'hard': self._hard_reload

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesn't look like the 'hard' variable is used at all in the front-end. Can we just remove it here, in _on_assets_change and in __init__?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh okay, I see. So is this variable just set to true on the first asset change? I see it gets set to True in _on_assets_change but don't see it getting set back anywhere

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good catch, need to reset it after serving the response.

Comment threaddash/dash.py Outdated
self.config.routes_pathname_prefix,
path)

def _on_assets_change(self, _):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why pass the argument here? Can't we just call on_change()

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The watch function I got here is from another project of mine that I adapted to run on threads instead of asyncio. When a file changed in the other project, I would log the filename thus giving the changed filename as argument in the callback. But we don't have a logging system in-place so I just put a pass param. I guess I could remove the options we're not gonna use here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh okay, didn't know thats what the argument was. I think we should keep it to add logging in the future, something like File <file_name> was changed at <time>. Maybe put a comment that says the _ is the file name in case another dev makes this change.

rmarren1
rmarren1 previously requested changes Sep 12, 2018

@rmarren1rmarren1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! These are mostly questions, and I think there are a few things you can delete

@chriddyp

chriddyp commented Sep 14, 2018

Copy link
Copy Markdown
Member

This is looking really great!
hot reloading demo

A few things I ran into:

  • If I add a new component library in layout, the JS bundle doesn't get served. Use case: start with an empty app.layout = html.Div([]) and then incrementally build the app. When I added dcc.Graph, it didn't appear until I refreshed.
  • Similarly, if I created a new CSS file, I had to restart the app before I saw it.

It seems like the developer-facing API is small (just the hot_reload tag), so we should be free to change the backend implementation in the future, right? In particular, I would be interested in exploring (in the future):

  • A websocket based approach instead of front-end timers.
  • An approach where CSS files are reloaded separately without a full page reload. Use case:
    • User could click through the application, getting it to a certain state where certain elements are being shown, and then style those elements with CSS without changing the state.
    • CSS updates could happen a lot quicker than the Python updates, since it doesn't (in theory) necessarily require a full refresh of the underling python code

@T4rk1n

Copy link
Copy Markdown
ContributorAuthor

If I add a new component library in layout, the JS bundle doesn't get served.

That's a hard one. Since it's a soft reload, it won't reload the index and the new resources doesn't get served.

The solution I am thinking of:

  1. Keep a list of the dependencies in a list in the store, available in a API.requestComponentsLib.
  2. Check the components libs scripts tags are all included in the requestComponentLib response, if not append it to the footer and wait for it to be loaded before triggering a soft reload.

if I created a new CSS file, I had to restart the app before I saw it.

I think that is related to #341. The assets folder is walked only once during initialization, my initial fix was to walk every time the index is served. But then I thought it would be better to integrate the watch of hot-reload and add it to the assets resources since that fix is not needed for deployments and incur a performance cost.

It seems like the developer-facing API is small (just the hot_reload tag),

There's also hot_reload_interval, and maybe watch_interval that can be added.

An approach where CSS files are reloaded separately without a full page reload.

Could do that by disabling the old stylesheets and append the new one with the modified timestamp in the head.

@chriddyp

Copy link
Copy Markdown
Member

If I add a new component library in layout, the JS bundle doesn't get served.

That's a hard one. Since it's a soft reload, it won't reload the index and the new resources doesn't get served.

Another approach might be to solve it by solving plotly/dash-renderer#46. I think that issue will be a lot more challenging to solve though.

@T4rk1n

Copy link
Copy Markdown
ContributorAuthor

@chriddyp

I solved it by including the keys of dash.registered_paths in the reload request and checking that the length and content is the same, if not hard reload. Was easier than I thought.

For plotly/dash-renderer#46, I think we would have to add a method to register the packages externally from the layout collection so they can be included even if not in the initial layout.

@T4rk1n

Copy link
Copy Markdown
ContributorAuthor

An approach where CSS files are reloaded separately without a full page reload

@chriddyp I added that functionality.

@T4rk1n
T4rk1nforce-pushed the hot-reload branch 2 times, most recently from a47c5d7 to 6736cc7CompareSeptember 21, 2018 18:33
@T4rk1n

Copy link
Copy Markdown
ContributorAuthor

Updated the rc versions:

dash==0.29.0rc3
dash-renderer==0.15.0rc2

@chriddyp

Copy link
Copy Markdown
Member

This latest version is looking really great. The new CSS pathway is especially nice 🎉
hot-reload-demo

@mbkupfer

Copy link
Copy Markdown

I'm having trouble with running this. When I start my app, it goes into an endless loop with the following error messages:


File "C:\Users\mkupfer\Desktop\MYDOCU~1\01_VIS~1\PAY-RA~1\venv\lib\site-packag
es\flask\app.py", line 1718, in handle_user_exception
reraise(exc_type, exc_value, tb)
File "C:\Users\mkupfer\Desktop\MYDOCU~1\01_VIS~1\PAY-RA~1\venv\lib\site-packag
es\flask\_compat.py", line 35, in reraise
raise value
File "C:\Users\mkupfer\Desktop\MYDOCU~1\01_VIS~1\PAY-RA~1\venv\lib\site-packag
es\flask\app.py", line 1813, in full_dispatch_request
rv = self.dispatch_request()
File "C:\Users\mkupfer\Desktop\MYDOCU~1\01_VIS~1\PAY-RA~1\venv\lib\site-packag
es\flask\app.py", line 1799, in dispatch_request
return self.view_functions[rule.endpoint](**req.view_args)
File "C:\Users\mkupfer\Desktop\MYDOCU~1\01_VIS~1\PAY-RA~1\venv\lib\site-packag
es\dash\dash.py", line 336, in serve_reload_hash
'files': list(changed)
File "C:\Users\mkupfer\Desktop\MYDOCU~1\01_VIS~1\PAY-RA~1\venv\lib\site-packag
es\flask\json\__init__.py", line 321, in jsonify
dumps(data, indent=indent, separators=separators) + '\n',
File "C:\Users\mkupfer\Desktop\MYDOCU~1\01_VIS~1\PAY-RA~1\venv\lib\site-packag
es\flask\json\__init__.py", line 179, in dumps
rv = _json.dumps(obj, **kwargs)
File "C:\Users\mkupfer\AppData\Local\Programs\Python\Python36-32\Lib\json\__in
it__.py", line 238, in dumps
**kw).encode(obj)
File "C:\Users\mkupfer\AppData\Local\Programs\Python\Python36-32\Lib\json\enco
der.py", line 201, in encode
chunks = list(chunks)
File "C:\Users\mkupfer\AppData\Local\Programs\Python\Python36-32\Lib\json\enco
der.py", line 430, in _iterencode
yield from _iterencode_dict(o, _current_indent_level)
File "C:\Users\mkupfer\AppData\Local\Programs\Python\Python36-32\Lib\json\enco
der.py", line 404, in _iterencode_dict
yield from chunks
File "C:\Users\mkupfer\AppData\Local\Programs\Python\Python36-32\Lib\json\enco
der.py", line 437, in _iterencode
o = _default(o)
File "C:\Users\mkupfer\Desktop\MYDOCU~1\01_VIS~1\PAY-RA~1\venv\lib\site-packag
es\flask\json\__init__.py", line 81, in default
return _json.JSONEncoder.default(self, o)
File "C:\Users\mkupfer\AppData\Local\Programs\Python\Python36-32\Lib\json\enco
der.py", line 180, in default
o.__class__.__name__)
TypeError: Object of type 'dict_keys' is not JSON serializable

@T4rk1n

Copy link
Copy Markdown
ContributorAuthor

@mbkupfer thanks for reporting, I think I know what's wrong, will post update soon.

@T4rk1n

Copy link
Copy Markdown
ContributorAuthor

@mbkupfer Released dash==0.29.0rc4, should fix the json issue on py3.

@T4rk1n

Copy link
Copy Markdown
ContributorAuthor

Had to remove the resources cache, a proper cache bust check was more expansive than remapping them.

@T4rk1n
T4rk1n merged commit f590166 into masterNov 14, 2018
@T4rk1n
T4rk1n deleted the hot-reload branch November 14, 2018 21:06
@chriddyp

Copy link
Copy Markdown
Member

Amazing work @T4rk1n ! Is there is an issue logged for the documentation items I mentioned in #362 (comment)?

@T4rk1n

Copy link
Copy Markdown
ContributorAuthor

@chriddyp There is an issue for the dev tools https://github.com/plotly/dash-docs/issues/196

Sign up for freeto 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

@T4rk1n@chriddyp@mbkupfer@rmarren1