Kronuz / pyScss

pyScss, a Scss compiler for Python
MIT License
582 stars 141 forks source link

Make an earier copy of the rules to close up the window for concurrent modification to break things. #304

Closed doug-fish closed 10 years ago

doug-fish commented 10 years ago

we've recently picked switched to SCSS in openstack Horizon and we are seeing some intermittent scss compile failures: https://bugs.launchpad.net/horizon/+bug/1345955. We think it's related to multiple threads attempting to compile the same scss files.

We are exploring ways to avoid the multithreading altogether, but it seems to me much safer if the compilation just worked. It seems that the compiler "apply_extends" method breaks if multiple threads run through it; but the fix seems as easy as just making a copy of self.rules at the start of the method, process the copy and set it back at the end.

Sharing my prototype code that fixes the problem in my dev environment.

eevee commented 10 years ago

Out of curiosity, does current master work any better? It splits the compiler away from a single compilation run, so there should be far less shared mutable state.

doug-fish commented 10 years ago

eevee, I had the same question you did and attempted that last Friday. I couldn't readily integrate master pyScss into Horizon, so I never really found the answer. I've seen the refactor that is in master and it does seem promising. I'll take another shot at getting that environment to run.

doug-fish commented 10 years ago

I've spent another 1/2 hour digging around in django-compressor and django-pyscss trying to get Horizon to run with master pyScss and haven't solved the issues.

Eevee, do you have mild, passing curiosity about this, or a level burning level of curiosity which might drive you to help me sort through debugging this environment? :-)

eevee commented 10 years ago

Well now I'm pretty curious why you're having such trouble with master, since I've been hoping to make a release based off of it real soon now :)

eevee commented 10 years ago

I'm okay with the merge, but definitely interested in what's causing problems with master; it's supposed to be backwards-compatible, save for some ancient cruft that's not even part of Sass.

doug-fish commented 10 years ago

Edited: Not sure what I was doing earlier, but I cannot run on master. I can run on 1.2.x, but when I checkout master I see this stack trace:

Internal Server Error: /admin/ Traceback (most recent call last): File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 137, in get_response response = response.render() File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/response.py", line 105, in render self.content = self.rendered_content File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/response.py", line 82, in rendered_content content = template.render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/base.py", line 140, in render return self._render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/base.py", line 134, in _render return self.nodelist.render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/base.py", line 840, in render bit = self.render_node(node, context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/debug.py", line 78, in render_node return node.render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/loader_tags.py", line 123, in render return compiled_parent._render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/base.py", line 134, in _render return self.nodelist.render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/base.py", line 840, in render bit = self.render_node(node, context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/debug.py", line 78, in render_node return node.render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/loader_tags.py", line 62, in render result = block.nodelist.render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/base.py", line 840, in render bit = self.render_node(node, context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/debug.py", line 78, in render_node return node.render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/loader_tags.py", line 155, in render return self.render_template(self.template, context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/loader_tags.py", line 137, in render_template output = template.render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/base.py", line 140, in render return self._render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/base.py", line 134, in _render return self.nodelist.render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/base.py", line 840, in render bit = self.render_node(node, context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/django/template/debug.py", line 78, in render_node return node.render(context) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/compressor/templatetags/compress.py", line 149, in render return self.render_compressed(context, self.kind, self.mode, forced=forced) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/compressor/templatetags/compress.py", line 108, in render_compressed rendered_output = self.render_output(compressor, mode, forced=forced) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/compressor/templatetags/compress.py", line 121, in render_output return compressor.output(mode, forced=forced) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/compressor/css.py", line 51, in output ret.append(subnode.output(_args, _kwargs)) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/compressor/css.py", line 53, in output return super(CssCompressor, self).output(_args, _kwargs) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/compressor/base.py", line 285, in output output = '\n'.join(self.filter_input(forced)) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/compressor/base.py", line 224, in filter_input for hunk in self.hunks(forced): File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/compressor/base.py", line 200, in hunks precompiled, value = self.precompile(value, options) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/compressor/base.py", line 254, in precompile return True, filter.input(kwargs) File "/home/drf/horizon/.venv/local/lib/python2.7/site-packages/compressor/filters/base.py", line 173, in input raise FilterError(err) FilterError: /bin/sh: 1: django_pyscss.compressor.DjangoScssFilter: not found

Honestly, I can't sort out how this is caused by pyScss, but when I checkout master for pyScss Horizon starts working again. Thoughts on how to debug further?

doug-fish commented 10 years ago

Here's another lead on potential problems .... does any of this usage concern/surprise you? https://github.com/fusionbox/django-pyscss/blob/stable/1.0.x/django_pyscss/scss.py

eevee commented 10 years ago

Oh dear. Overriding Scss._do_import is not likely to work against master. I'll have to come up with a better import-hooking API (there's a ticket to do that somewhere) before a release, and send them a PR...

doug-fish commented 10 years ago

I've pulled out pdb and have another observation (Pdb) from scss import Scss *\ ImportError: No module named enum

I think that's from this line, right? https://github.com/Kronuz/pyScss/blob/d1a5e27c5e01e5a7d6c0df8a5a4a828459dbcb39/scss/compiler.py#L7

I'm running python 2.7 - I think that's going to cause me trouble, right? -- Enum is new https://docs.python.org/3/library/enum.html

eevee commented 10 years ago

enum34 should be listed in setup.py; it's a backport.

eevee commented 10 years ago

I have absolutely no idea what's causing your long backtrace, though; all of that looks like Django stuff, which I'm not very familiar with.

I've patched master to fix this issue, I hope. (Same general approach, but pulled the self. concerns out of the method entirely.)

eevee commented 10 years ago

Also there's some conversation about dealing with django-pyscss's needs in #265 if you're interested.