appsembler / figures

Reporting and data retrieval app for Open edX
MIT License
44 stars 37 forks source link

Planning Figures Juniper upgrade (Python 3, Django 2) #241

Closed johnbaldwin closed 2 years ago

johnbaldwin commented 4 years ago

Here are my thoughts for doing the Figures Juniper upgrade:

Overview

The core parts of Figures:

And then there are the configuration files for pytest, tox, codecov in the project root

Suggestion for approach

We would like to do this incrementally with a number of small PRs rather than a huge PR.

First, this is the first real effort for community contribution to Figures. I want to make sure community members get the most value for their development time. Starting small means we can have "small miscommunications" instead of "big miscommunications" and reduce risk of rejecting contributions because of a misunderstanding.

Second, we must maintain backward compatibility. We still have systems on Ginkgo and Hawthorn running Figures. I am NOT maintaining multiple branches for different versions.

Over my career, I've learned a single codebase to support multiple targets is more maintainable and less costly in the long run than keeping different codebases synchronized. Where one finds there are hard incompatibilities that necessitate branching, that is a cue to pull that incompatible functionality out of the core code and into an adapter/plugin.

Steps

I suggest we approach the upgrade in the following sequence so we can "layer in" Juniper support with minimal initial changes to existing files:

  1. Add a new ./devsite/requirements/juniper_community.txt Pip requirements file.

Copy the hawthorn_community.txt file and update versions for those in Juniper. There might be new packages, add those, there might be packages no longer needed. We can discuss as we find those

  1. Create a pytest-juniper.ini file in the project root, just like we have a special pytest.ini file for Ginkgo, we'll start with one for Juniper.

See the Ginkgo file for reference: https://github.com/appsembler/figures/blob/master/pytest-ginkgo.ini

  1. Create Juniper mocks

3.a. Create a new ./mocks/juniper/ directory, copying from ./mocks/hawthorn

3.b. Review platform changes made to the models and supporting code in the mocks.

For example, look over the student module in Figures mocks (https://github.com/appsembler/figures/blob/master/mocks/hawthorn/student/models.py) What are the changes made in the Juniper upgrade in the platform?

https://github.com/edx/edx-platform/blob/open-release/juniper.master/common/djangoapps/student/models.py

We really only want to mock what we absolutely need to

The juniper mocks is probably going to be one of the bigger tasks to do for the Django 2/Python 3 upgrade. And although this is a lot of effort, I find the mocks to be very well worth the effort. It helps speed up development and makes integration with the platform more purposeful.

Question: Why not just mock everything in the unit tests with Mock? Answer: The platform mocks let us run Figures development server so we can interact and explore Figures standalone without requiring the platform to be installed on the developer's system. There are other reasons, but that's a key one.

  1. Update test settings (optionally also update devsite development server settings settings too

Up for discussion on the approach here. At a minimum, we need to update ./devsite/devsite/test_settings.py or we need to create a new test settings file (copy existing test_settings.py and modify) for Juniper testing.

It might be helpful to just update the whole devsite so we have a working sandbox for Figures. Note we need to maintain backward compatibility.

Updating settings:

We have two basic options here:

Option A. Add a new juniper_settings.py for the development web server and juniper_test_settings.py for the pytest unit tests

Option B. Update the existing settings.py and test_settings.py files to support Juniper while maintaining backward compatibility

Let's discuss which approach when we see what needs to be changed. What I suggest is we do a draft PR modifying the existing settings to identify the required changes. From this PR, we'll know which direction we want to go in for maintainability.

  1. Add a Juniper environment to ./tox.ini

https://github.com/appsembler/figures/blob/master/tox.ini#L31-L39

  1. Update ./figures/ and ./tests

IMPORTANT: We must maintain backward compatibility. We use the figures.compat module to serve as a compatibility layer to abstract platform release specifics from the rest of Figures code. See how it currently works:

https://github.com/appsembler/figures/blob/master/figures/compat.py

Front end

The frontend code has no Python and therefore should not need to be updated for doing a Python 3 upgrade

BbrSofiane commented 4 years ago

Hi @johnbaldwin,

Thanks for the thorough breakdown!

3. Create Juniper mocks

4. Update settings

5. Add Juniper environment to ./tox.ini

BbrSofiane commented 4 years ago

Also do you take PRs from forks? Or would that work be better in a branch?

johnbaldwin commented 4 years ago

@BbrSofiane Fork, please. This will be a first time for me to accept PRs from forks, but since it is standard practice for open source communities, I should get to know it. So I'll practice it on my own so I've got the steps right

BbrSofiane commented 4 years ago

@johnbaldwin Should I create a PR against your master branch or do you want to create a specific branch?

rmaunder commented 3 years ago

Hi @johnbaldwin Do you have any update / prediction on when the Juniper migration might be ready for use?

Thanks!

rmaunder commented 3 years ago

Great to see. Thanks for all the work on getting to this stage. Look forward to trying it out!

johnbaldwin commented 3 years ago

Status update: The Juniper upgrade is in Figures master branch and slated for release 0.4.0, which I'm working on this week

johnbaldwin commented 2 years ago

Ok, stale issue. Figures supports Juniper. I'll make an effort keep up better on maintaining state of these issues