QOL-9055 add Python 3 to CI testing and fix compatibility #72
Conversation
ThrawnCA
commented
Jul 18, 2022
ThrawnCA
commented
- Add Github Actions run for Python 3
- Replace Fanstatic with Webassets on CKAN 2.9+
- Replace deprecated 'cgi.escape' if 'html.escape' is available
- Fix Flask routing for dataset reads
- we need to be consistent about using the 'ckan-' prefix or not, but it's more flexible not to assume it
- 'html.escape' appears to be available on Py2 as well, so we can use that
- Use Fanstatic for older CKAN, Webassets for CKAN 2.9+
- use 'dataset.read' not 'package.read', and use 'dataset_read' on older CKAN
ThrawnCA
requested review from
duttonw,
devonpis,
kundupz,
asifaminb,
chris-randall-qol and
RossWebsterWork
| cmd: | | ||
| docker cp . $(docker-compose ps -q ckan):/app/ | ||
| docker cp .docker/scripts/ckan_cli $(docker-compose ps -q ckan):/app/ckan/default/bin/ | ||
| ahoy cli 'chmod u+x $VENV_DIR/bin/ckan_cli; cp .docker/test.ini $CKAN_INI' |
There was a problem hiding this comment.
why mix, doesn't docker have a cli interface also under the ahoy hood?
There was a problem hiding this comment.
The Ahoy task takes care of identifying the 'ckan' container. It's not a big deal, but it already exists, so may as well use it.
| ckan-version: [ckan-2.8.8, ckan-2.9.5] | ||
| python-version: [py2, py3] | ||
| exclude: | ||
| - ckan-version: ckan-2.8.8 |
| @@ -0,0 +1,15 @@ | |||
| edit_comments-js: | |||
There was a problem hiding this comment.
how does this work, link to doc's for others to learn also please
There was a problem hiding this comment.
Added a comment linking to https://docs.ckan.org/en/2.9/contributing/frontend/assets.html
| @@ -54,13 +54,13 @@ def get_blueprint(self): | |||
| ('GET', 'POST',), | |||
| ), | |||
| ( | |||
| "/{}/follow/<datarequest_id>".format(constants.DATAREQUESTS_MAIN_PATH), | |||
| "/{}/follow/<id>".format(constants.DATAREQUESTS_MAIN_PATH), | |||
There was a problem hiding this comment.
isn't id too generic, why did we need to shrink it?
There was a problem hiding this comment.
It has to match what's actually used in the page, otherwise there's an error. We could change it, but would then also need to change the Pylons routing, which just uses 'id'.
| @@ -39,7 +40,7 @@ <h3>{{ _('Additional Info') }}</h3> | |||
| <tr> | |||
| <th scope="row" class="dataset-label">{{ _('Accepted dataset') }}</th> | |||
| <td class="dataset-details"> | |||
| {% link_for datarequest.accepted_dataset['title'], named_route='package.read', id=datarequest.accepted_dataset.get('id') %} | |||
| {% link_for datarequest.accepted_dataset['title'], named_route=dataset_read_route, id=datarequest.accepted_dataset.get('id') %} | |||
