✨(lti) add base Django app service #30

jmaupetit · 2023-06-23T15:23:06Z

Purpose

We need to serve the frontend application via LTI.

Proposal

add base LTI authentication application
add lint/test running utilities
configure the CI

Makefile

src/app/.pylintrc

src/app/pyproject.toml

lebaudantoine

Aaaaamaazing 👏

src/app/apps/lti/apps.py

docker-compose.yml

Makefile

docker-compose.yml

src/app/pyproject.toml

src/app/warren/wsgi.py

src/app/apps/lti/forms.py

src/app/.pylintrc

lebaudantoine · 2023-07-13T20:13:44Z

Regarding Marsha's Makefile, I've noticed they provide additional features beyond what I've included.

If they are relevant, would you like me to include these for improving the developer experience in this PR?

lebaudantoine · 2023-07-13T20:18:13Z

I have reused ruff configuration from src/backend.
However, It appears that this pylint check is not present or missing:

import-outside-toplevel (C0415)
Used when an import statement is used anywhere other than the module toplevel. Move this import to the top of the file.

Do we ignore it?

lebaudantoine · 2023-07-13T20:20:59Z

Note: The unused imports from empty example files in the apps/development have been suffixed with # noqa: F401 to keep them. ex apps/development/admin.py:

"""Admin for the dashboard app."""
from django.contrib import admin  # noqa: F401

# Register your models here.

lebaudantoine · 2023-07-13T20:28:49Z

During your review, could you please pay special attention to the following:

Updates to CI configurations.
Updates to the Makefile, particularly the changes made to the test execution process for backend and app.

Currently, the pytest command causes the CI job test-app to fail because it cannot collect any tests. This is expected since no tests have been written yet. I consider implementing one of these approaches to address the issue:

Writing at least one test to ensure the CI job can pass.
Updating the CI configuration to handle the scenario of no tests available.

WDYT?

jmaupetit · 2023-07-17T09:12:44Z

Currently, the pytest command causes the CI job test-app to fail because it cannot collect any tests. This is expected since no tests have been written yet. I consider implementing one of these approaches to address the issue:
* Writing at least one test to ensure the CI job can pass.

* Updating the CI configuration to handle the scenario of no tests available.
WDYT?

I would add a first test.

jmaupetit

Looks great, thanks!

Waiting for a first test to be added for the app and then we'll be good to go.

Note that we now have a dependency issue for backend tests.

src/app/warren/settings.py

src/app/warren/wsgi.py

.circleci/config.yml

lebaudantoine · 2023-07-17T22:49:04Z

The missing pylint import-outside-toplevel (C0415) check should be shipped soon. A PR has been approved on the repo. @jmaupetit

jmaupetit

I can't approve this PR since I opened it, but, we are good to go 🎉

Warren core application is a Django project that will serve Dashboards exclusively via LTI in its first release.

Introduce ruff and black to lint Python sources of the app. Ruff's configurations are on pair with the backend. Offer developers make commands that allow them linting the app or the backend either independently or together.

The majority of linter errors were related to missing or incorrectly formatted docstrings in the `src/app`. Ruff `noqa` comments have been added to some `src/app/dashboard` Python sources, yet empty. It's temporary, these sources will be updated in a near future. They serve as examples and should not be removed.

Offer developers make commands that allow them running any test on the `src/app` or the `src/backend` Python sources, either independently or together.

Automate checks on building, linting and testing production and development `src/app` Docker image.

Make core and plugin pyproject files consistent with python images being built by the circle ci jobs (3.8, 3.9, 3.10).

Add a simple test on the development view to make the CI passes. Without any test added, test-app job was resulting in failure.

Inspired by marsha project, this update introduces new commands that assist developers in generating migrations and verifying Django sources.

jmaupetit added feature WIP labels Jun 23, 2023

jmaupetit self-assigned this Jun 23, 2023

jmaupetit requested a review from lebaudantoine June 23, 2023 15:23

jmaupetit commented Jun 23, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

src/app/.pylintrc Outdated Show resolved Hide resolved

src/app/pyproject.toml Show resolved Hide resolved

jmaupetit marked this pull request as draft June 23, 2023 15:27

lebaudantoine approved these changes Jun 28, 2023

View reviewed changes

jmaupetit commented Jun 30, 2023

View reviewed changes

src/app/.pylintrc Outdated Show resolved Hide resolved

lebaudantoine force-pushed the add-django-lti-app branch 6 times, most recently from bc70848 to 4f498e3 Compare July 13, 2023 19:34

lebaudantoine force-pushed the add-django-lti-app branch 4 times, most recently from 4597944 to 8f56054 Compare July 13, 2023 21:00

jmaupetit commented Jul 17, 2023

View reviewed changes

src/app/warren/settings.py Show resolved Hide resolved

src/app/warren/wsgi.py Outdated Show resolved Hide resolved

.circleci/config.yml Outdated Show resolved Hide resolved

.circleci/config.yml Outdated Show resolved Hide resolved

lebaudantoine force-pushed the add-django-lti-app branch 5 times, most recently from e02d82d to c6cbce6 Compare July 17, 2023 22:44

lebaudantoine marked this pull request as ready for review July 17, 2023 23:27

lebaudantoine force-pushed the add-django-lti-app branch 2 times, most recently from c669f67 to 650d7e1 Compare July 17, 2023 23:32

jmaupetit commented Jul 18, 2023

View reviewed changes

jmaupetit and others added 8 commits July 18, 2023 15:20

✨(lti) add base Django app service

55bfbe2

Warren core application is a Django project that will serve Dashboards exclusively via LTI in its first release.

👷(lti) implement linting for all Python sources

c9e659f

Introduce ruff and black to lint Python sources of the app. Ruff's configurations are on pair with the backend. Offer developers make commands that allow them linting the app or the backend either independently or together.

👷(lti) implement new testing make commands

a8270b4

Offer developers make commands that allow them running any test on the `src/app` or the `src/backend` Python sources, either independently or together.

👷(lti) implement circle CI jobs on src/app

ffc5158

Automate checks on building, linting and testing production and development `src/app` Docker image.

🐛(backend) align pyproject classifiers with ci jobs

1a97968

Make core and plugin pyproject files consistent with python images being built by the circle ci jobs (3.8, 3.9, 3.10).

✅(lti) add a first test

ba34fb6

Add a simple test on the development view to make the CI passes. Without any test added, test-app job was resulting in failure.

👷(lti) include Django related make commands

93ef848

Inspired by marsha project, this update introduces new commands that assist developers in generating migrations and verifying Django sources.

lebaudantoine force-pushed the add-django-lti-app branch from 650d7e1 to 93ef848 Compare July 18, 2023 13:20

lebaudantoine merged commit f431a84 into main Jul 18, 2023

lebaudantoine deleted the add-django-lti-app branch July 18, 2023 13:23

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨(lti) add base Django app service #30

✨(lti) add base Django app service #30

jmaupetit commented Jun 23, 2023 •

edited by lebaudantoine

Loading

lebaudantoine left a comment •

edited

Loading

lebaudantoine commented Jul 13, 2023

lebaudantoine commented Jul 13, 2023

lebaudantoine commented Jul 13, 2023 •

edited

Loading

lebaudantoine commented Jul 13, 2023 •

edited

Loading

jmaupetit commented Jul 17, 2023

jmaupetit left a comment

lebaudantoine commented Jul 17, 2023 •

edited

Loading

jmaupetit left a comment

✨(lti) add base Django app service #30

✨(lti) add base Django app service #30

Conversation

jmaupetit commented Jun 23, 2023 • edited by lebaudantoine Loading

Purpose

Proposal

lebaudantoine left a comment • edited Loading

Choose a reason for hiding this comment

lebaudantoine commented Jul 13, 2023

lebaudantoine commented Jul 13, 2023

lebaudantoine commented Jul 13, 2023 • edited Loading

lebaudantoine commented Jul 13, 2023 • edited Loading

jmaupetit commented Jul 17, 2023

jmaupetit left a comment

Choose a reason for hiding this comment

lebaudantoine commented Jul 17, 2023 • edited Loading

jmaupetit left a comment

Choose a reason for hiding this comment

jmaupetit commented Jun 23, 2023 •

edited by lebaudantoine

Loading

lebaudantoine left a comment •

edited

Loading

lebaudantoine commented Jul 13, 2023 •

edited

Loading

lebaudantoine commented Jul 13, 2023 •

edited

Loading

lebaudantoine commented Jul 17, 2023 •

edited

Loading