Skip to content
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

2.9.1 incompatibility with flask admin #642

Closed
bolkedebruin opened this issue Jan 7, 2017 · 12 comments

Comments

@bolkedebruin
Copy link

commented Jan 7, 2017

We have jinja2 in Airflow and it throws an error that seems tied to our usage of flask-admin:

======================================================================
ERROR: test_dag_views (tests.WebUiTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/apache/incubator-airflow/tests/core.py", line 1423, in test_dag_views
    '/admin/airflow/graph?dag_id=example_bash_operator')
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/werkzeug/test.py", line 778, in get
    return self.open(*args, **kw)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/testing.py", line 127, in open
    follow_redirects=follow_redirects)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/werkzeug/test.py", line 751, in open
    response = self.run_wsgi_app(environ, buffered=buffered)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/werkzeug/test.py", line 668, in run_wsgi_app
    rv = run_wsgi_app(self.application, environ, buffered=buffered)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/werkzeug/test.py", line 871, in run_wsgi_app
    app_rv = app(environ, start_response)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/app.py", line 1994, in __call__
    return self.wsgi_app(environ, start_response)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/app.py", line 1985, in wsgi_app
    response = self.handle_exception(e)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/app.py", line 1540, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/app.py", line 1982, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/app.py", line 1614, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/app.py", line 1517, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/app.py", line 1612, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/app.py", line 1598, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask_admin/base.py", line 69, in inner
    return self._run_view(f, *args, **kwargs)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask_admin/base.py", line 368, in _run_view
    return fn(self, *args, **kwargs)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask_login.py", line 755, in decorated_view
    return func(*args, **kwargs)
  File "/home/travis/build/apache/incubator-airflow/airflow/www/utils.py", line 221, in view_func
    return f(*args, **kwargs)
  File "/home/travis/build/apache/incubator-airflow/airflow/www/utils.py", line 125, in wrapper
    return f(*args, **kwargs)
  File "/home/travis/build/apache/incubator-airflow/airflow/www/views.py", line 1438, in graph
    edges=json.dumps(edges, indent=2),)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask_admin/base.py", line 308, in render
    return render_template(template, **kwargs)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/templating.py", line 134, in render_template
    context, ctx.app)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/flask/templating.py", line 116, in _render
    rv = template.render(context)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/jinja2/environment.py", line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/jinja2/environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/home/travis/build/apache/incubator-airflow/airflow/www/templates/airflow/graph.html", line 19, in top-level template code
    {% import 'admin/lib.html' as lib with context %}
  File "/home/travis/build/apache/incubator-airflow/.tox/py27-cdh-airflow_backend_sqlite/lib/python2.7/site-packages/jinja2/environment.py", line 551, in _compile
    return compile(source, filename, 'exec')
SyntaxError: duplicate argument 'l_1_caller' in function definition (lib.html, line 255)
@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Jan 7, 2017

Looks like a regression. Until it's fixed I'd recommend requiring jinja2<2.9 in your package requirements

@bolkedebruin

This comment has been minimized.

Copy link
Author

commented Jan 7, 2017

We are now. Thanks.

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented Jan 7, 2017

Would it be possible to get a minimal repro case for this?

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented Jan 7, 2017

Found a minimal repo. Interesting that this worked before:

{% macro x(caller=none) -%}
    [{% if caller %}{{ caller() }}{% endif %}]
{%- endmacro %}

{{ x() }}
{% call x() %}aha!{% endcall %}

Going to investigate what this did.

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Jan 7, 2017

isn't that caller=none arg pointless anyway? caller is already none (or undefined?) when calling a macro without {% call %}

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented Jan 7, 2017

@ThiefMaster it's undefined and yes. I assume the correct behavior will be to just silently ignore caller here. However I wonder if you could set it to a different default before.

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented Jan 7, 2017

Eg. Could you do {% macro x(caller=42) %}...{% endmacro %}.

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Jan 7, 2017

sounds like something that should at least trigger a warning

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented Jan 7, 2017

So you could define a default in the past but only if caller was the last argument I think. If you did {% macro x(caller, foo, bar) %} then it would fail when used with {% call %}.

@mitsuhiko

This comment has been minimized.

@mitsuhiko mitsuhiko closed this in 6235644 Jan 8, 2017

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented Jan 8, 2017

Not sure if that change is good enough to cover all cases but this is a change I am willing to make. This at least gives it a clear behavior now.

@bolkedebruin

This comment has been minimized.

Copy link
Author

commented Jan 8, 2017

That was quick! Thanks, will verify our own code whether we do something "unexpected".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.