GSA / datagov-ckan-multi

Other
10 stars 6 forks source link

ckanext-datajson Py3 + CKAN 2.9 support #564

Closed thejuliekramer closed 3 years ago

thejuliekramer commented 3 years ago

User Story

As a data.gov developer, I want ckanext-datajson running with CKAN 2.9 and Python 3 so that we can move out of CKAN 2.8 and reduce our technical debt and meet compliance standards.

Acceptance Criteria

Background

CKAN 2.9 extension tracker.

Details / tasks

The goal is to support running this extension against CKAN 2.9 (Python 3) and CKAN 2.8 (Python 2) environments. Please refer to the main CKAN documentation for Python 3 extensions migration. We're not following this guide exactly. Instead of creating a long-lived py3 branch, we should be merging often. New CI test suites don't need to required or to be fully passing in order to merge. It's only important that changes don't introduce new failures or break the existing test suites.

Below are a list of tasks. Depending on the extension's functionality, not all tasks will be actionable.

Tasks:

Additional resources:

adborden commented 3 years ago

Updated the issue description.

adborden commented 3 years ago

caniusepython3 output. Take this with a grain of salt because I don't think our requirements.txt is a complete view of the extensions' dependencies.

$ pipenv run caniusepython3 -r requirements.txt
Loading .env environment variables...
Finding and checking dependencies ...

You need 1 project to transition to Python 3.
Of that 1 project, 1 has no direct dependencies blocking its transition:

  lepl
adborden commented 3 years ago

Default branch has been renamed. Don't forget to run:

git branch -m master main
git fetch origin
git branch -u origin/main main
git remote set-head origin -a
adborden commented 3 years ago

@FuhuXia I guess there's some flakiness in the CircleCI tests. It looks like main is mostly passing minus the test suites that we're planning on removing anyway.

adborden commented 3 years ago

@FuhuXia I've got 2 PRs for you:

Please check them out and merge them if ready. I'll sync up with you tomorrow.

adborden commented 3 years ago
nickumia-reisys commented 3 years ago

On branch feature/py3, re-ran the caniusepython3 command and it returned this..

$ python3 -m  caniusepython3 -r requirements.txt
[WARNING] Skipping '-e git+https://github.com/ckan/ckanext-harvest.git': could not parse requirement
Finding and checking dependencies ...

You need 2 projects to transition to Python 3.
Of those 2 projects, 2 have no direct dependencies blocking their transition:

  ckantoolkit
  lepl
jbrown-xentity commented 3 years ago

We have decided that the 2 blocking tests are better implemented as integration tests on catalog and inventory respectively. We will remove these unit tests, and this should be ready for merging. Deployment to production will be dependent on implementing the integration tests:

nickumia-reisys commented 3 years ago

There was a bug found related to changes in this upgrade; however, that bug was not being explicitly tested for and we are considering ckanext-datajson py3 upgrade complete. It runs on py3 and will continue to have bugs pop up as every code does.