[ckan-changes] [ckan/ckan] 2df141: Merge branch '3196-common-session' into 3196-commo...

GitHub noreply at github.com
Thu Aug 25 12:54:09 UTC 2016


  Branch: refs/heads/3196-common-url_for
  Home:   https://github.com/ckan/ckan
  Commit: 2df141fba3e672967066d104417cb010aea7a23c
      https://github.com/ckan/ckan/commit/2df141fba3e672967066d104417cb010aea7a23c
  Author: amercader <amercadero at gmail.com>
  Date:   2016-08-25 (Thu, 25 Aug 2016)

  Changed paths:
    M ckan/common.py
    M ckan/config/middleware/flask_app.py
    M ckan/lib/base.py
    A ckan/templates/tests/flash_messages.html
    A ckan/tests/config/test_sessions.py
    M ckan/tests/legacy/lib/test_i18n.py
    M setup.py

  Log Message:
  -----------
  Merge branch '3196-common-session' into 3196-common-url_for

Conflicts:
	ckan/lib/base.py


  Commit: 19f2be8e32cb3ab9534a4d66685665c7db6e00ce
      https://github.com/ckan/ckan/commit/19f2be8e32cb3ab9534a4d66685665c7db6e00ce
  Author: amercader <amercadero at gmail.com>
  Date:   2016-08-25 (Thu, 25 Aug 2016)

  Changed paths:
    M ckan/config/middleware/__init__.py

  Log Message:
  -----------
  [#3196] Provide the necessary environment for routers to work

Although each app has its own routing mechanism (Flask and Pylons
(routes)), we still need to be able to generate a URL that is served
by the other application. For instance, when requesting `/dataset`, which
is currently served by Pylons, we call `h.url_for(controller='api',
action='get_api')`. This endpoint is served by Flask, so Pylons won't
know how to generate the URL. We need a wrapper that tries Flask first
and falls back to Pylons (next commit). In order to do that though, the
mappers need to work even if the request is not served by their app.

For Flask routes under requests served by Pylons, we wrap the response
in a `test_request_context`. We don't use an `app_context` because this
causes the following errors:

    RuntimeError: Application was not able to create a URL adapter for
request independent URL generation.

For Pylons routes under requests served by Flask, we set the Routes
configuration directly.


  Commit: b5ea2e7ffa312534017275337c3d709d3a0a5e7b
      https://github.com/ckan/ckan/commit/b5ea2e7ffa312534017275337c3d709d3a0a5e7b
  Author: amercader <amercadero at gmail.com>
  Date:   2016-08-25 (Thu, 25 Aug 2016)

  Changed paths:
    M ckan/lib/helpers.py
    M ckan/tests/config/test_middleware.py
    M ckan/tests/lib/test_helpers.py

  Log Message:
  -----------
  [#3196] New version of url_for to supports both Flask and Pylons

Essentially this calls the Flask url_for function and falls back to
Pylons if a route could not be created.

There's a lot of parameters handling before calling the Flask router
to support Pylons syntax (at least the most common cases):

    # Pylons
    url = url_for(controller='api', action='action', ver=3,
            qualified=True)

    # Flask
    url = url_for('api.action', ver=3, _external=True)

We don't rely on the host generated by Flask, as SERVER_NAME might not
be set or might be not be up to date (as in tests changing
`ckan.site_url`). Contrary to the Routes mapper, there is no way in
Flask to pass the host explicitly, so we rebuild the URL manually
based on `ckan.site_url`, which is essentially what we alredy do on Pylons.

I experimented with automatically set SERVER_NAME based on `ckan.site_url`,
but decided against it. Apart from being magic that we want to avoid, the main
cause of concern of doing this is that when SERVER_NAME is set, the Flask
router requires the incoming URL (HTTP_HOST on environ) to have the same host
 as the one defined in SERVER_NAME, otherwise it won't match the URL and
return a 404. For example if ckan.site_url is set to http://localhost:5000
(so SERVER_NAME is localhost:5000), calls to http://127.0.0.1:5000 or
http://0.0.0.0:5000 won't work.

This would cause massive confusion to users, specially during
development or early deployment stages, so it's best to avoid it.


Compare: https://github.com/ckan/ckan/compare/2df141fba3e6^...b5ea2e7ffa31


More information about the ckan-changes mailing list