search

girder / girder_worker

Distributed task execution engine with Girder integration, developed by Kitware
http://girder-worker.readthedocs.io/
Apache License 2.0
34 stars 30 forks source link

Add support for Python 3 #146

Open kotfic opened 7 years ago

kotfic commented 7 years ago

Deliverables

cjh1 commented 7 years ago

119

kotfic commented 6 years ago

Background

WIth the merging of https://github.com/girder/girder_worker/pull/225 and https://github.com/girder/girder_worker/pull/228 We now have

  1. modern infrastructure a tox/pytest based test runner for girder worker's celery refactor code.
  2. legacy infrastructure a cmake/ctest based test runner for girder worker's legacy code.
  3. integration infrastructure an integration test framework based on pytest and docker.

Travis runs both modern and legacy tests. CircleCI runs the integration tests. The tests/ folder contains the modern tests, and in tests/integration/ the integration tests Legacy tests are now in girder_worker/core/tests/ and girder_worker/plugins/*/tests/

Practically this means almost all code for supporting the legacy implementation (both code and tests) are contained in girder_worker/core/ and girder_worker/plugins/. Do not try to add Python3 support to any of the code in those directories.

Here are the steps I would take:

Transition the celery refactor to python 2/3

  1. Currently there is a tox environment for Python 2.7 (along with linting and coverage). I would add a python 3 testing environment. and
  2. Run tox and fix issues until all tests pass. (you may also want to run sixer to get started here)

Add Python 3 integration tests

  1. Add a feature to disable the core tasks with an environment variable. Currently to support backwards compatibility by default girder-worker includes the girder_worker.run core task. If you include this task it will import code from inside core & plugins and under python3 the command will fail. This can be disabled by changing the core_tasks option in worker.dist.cfg (or worker.local.cfg) to false. Rather than hacking this this in the CI steps, I recommend adding a small feature here that allows overriding the config file with an environment variable. Eventually the girder python3 tests will want to leverage this environment variable to do the same thing. Ultimately it will go away once core/ and plugin/ code has been removed from the repository.

  2. Add a servicess to the docker_compose to the docker-compose.yml file for girder3 and girder_worker3, edit the entrypoint etc to use the Python3 executable instead of the default python (This may require changing the Dockerfile.girder and Dockerfile.girder_worker files to ensure necessary python3 libraries are installed, for example pip3).

  3. Add code that only adds 'Traditional' integration test endpoints in python2 environment here and here

  4. Add pytest marks to tests in integration/tests/ that hit the 'traditional' endpoint so that these tests can be easily excluded on the command line (Note that some tests are parametrized but that you can mark individual parameters function)

  5. Change the Makefile to initially only bring up all services but the girder3 and girder_worker3 service.

  6. Change the Makefile to include a test3 target that brings down the girder and girder_worker service, brings up the girder3 and girder_worker3 service, and executes pytest with the correct command line to ignore the python2 only 'traditional' tests.

  7. Fix until all integration tests pass.

zachmullen commented 6 years ago

@kotfic to disable legacy testing in travis, is removal of the cmake and ctest commands from the travis.yml file sufficient? It looks like tox also does linting and coverage, so I'm assuming that should work?

zachmullen commented 6 years ago

TODO

kotfic commented 6 years ago

tox linting and coverage just lints and covers the code that is not in the girder_worker/core/ and girder_worker/plugins/ folders. Removal from travis will essentially make all code in those folders untested, unlinted and uncovered. Setting core_tasks=False will also make all code in those folders unimported when executing girder-worker

kotfic commented 6 years ago

An alternate approach would be to move tox code into the circleci, then all legacy code would be tested on travis, all modern code would be tested on circle. That actually might be the best approach.

(With the execption of the 'traditional' integration tests which make sure changes in the modern code base do not break backwards compatability inside girder_worker.run)

zachmullen commented 6 years ago

Ok, I'll try that. After messing with travis for the last 15 minutes I think I've had my fill of it anyway.