Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite backend with Django #268

Merged
merged 42 commits into from
Dec 31, 2020
Merged

Conversation

jdemaeyer
Copy link
Collaborator

@jdemaeyer jdemaeyer commented Oct 18, 2020

@ubergesundheit @bCyberBasti @ThorbenJensen

This is a complete rewrite of the backend using Django and Django Rest framework instead of Peewee and FastAPI, including OpenAPI spec generation.

To Do:

  • Check Dockerfile adjustments make sense and are deployable (may I offload this to you @ubergesundheit?)
  • Adjust deployment for static file serving (needed by the web frontend of REST framework and by Django admin)
  • Add minDate, maxDate filters for events endpoint (making sure they're picked up by the OpenAPI spec generator)
  • Add location and organizer filters for events endpoint
  • Update README
  • Add Django admin functionality

Closes #250

@ubergesundheit
Copy link
Member

ubergesundheit commented Oct 19, 2020

Wow! Thanks for this serious effort! I'll take a look at the container image 👍

May I add these points/open questions to the To do list:

  • The README of the backend needs to be changed
  • Especially: Did something change with the execution of the scrapers?

@jdemaeyer
Copy link
Collaborator Author

You may add them but then they will be on there twice :P Scrapers are now run via ./manage.py crawl

@ubergesundheit
Copy link
Member

🤦 I apologize for not reading through until the end

So here are (I think) some real additional TODOs:

  • Add DJANGO_SECRET_KEY and DJANGO_HOSTS (maybe even more from settings.py) env to ansible (@bCyberBasti)
  • Update CI config to use changed commands (@ubergesundheit)
  • The old container did bind itself to 0.0.0.0. Is this preserved in the new version?
  • Think about the new entrypoint script of the container image.. I'm not sure if we want to run database migration on every container start.

@jdemaeyer
Copy link
Collaborator Author

I'm hoping to get around to finishing the rewrite sometime this weekend...

@jdemaeyer jdemaeyer changed the title WIP: Rewrite backend with Django Rewrite backend with Django Nov 4, 2020
@jdemaeyer
Copy link
Collaborator Author

Aaalrighty, this is ready from a "backend code" side! I have implemented everything I wanted to but there are still some tasks required to make this production-ready.

@ubergesundheit @bCyberBasti please take over these (or let me know what exactly you need from me):

Deployment

  • Tasks from @ubergesundheit above (I have already adjusted the Django container to bind to all addresses)
  • Set up static file generation and serving (roughly: we will need to run collectstatic somewhere inside the Django container, e.g. in serve.sh, configure the output directory of that to be a Docker volume, and make sure we serve files from this volume through a proper webserver, example setup)

Frontend Builds

Although the API responses look the same, the auto-generated OpenAPI spec between the old and new implementations seems different enough to break building the frontend. I don't know enough about the frontend to figure out how severe this is.
0c77151 is the commit that updates the API client and breaks the build. Looking at the diff of the two generated openapi.jsons is probably a good idea but I don't have time to look into that this week, probably not even next week. :/

Admin

I have added some basic Django admin config that allows editing events but is not very pretty. I have also added functionality to preserve edits across crawls and to hide events in a9db02f. It allows marking fields of an event as "dirty" (e.g. after manually editing the value), which will prevent the crawlers from overriding that field. That could serve as a poor man's implementation for some of the issues @ThorbenJensen outlined in #267.

@ubergesundheit
Copy link
Member

Sorry for my delayed reaction, but I took some time off. Thus said, thank you very much thus far. I'll now try to resolve the outstanding issues. Starting with the generated frontend client.

@ubergesundheit
Copy link
Member

@jdemaeyer Is the openapi generation under your control?

  • do you think it is possible to teach django to include some basic error (404,400) cases in the openapi json?
  • IMO the previous is related. Seems the openapi spec does not define any response property as required. This results that the generated TypeScript client interfaces define all response properties as such. Meaning additional checks for non null will be required in code in many places. I have to research if this can be done in the frontend, but if its easy to do in django, please do. You can also point me to relevant docs

@ubergesundheit
Copy link
Member

Quick update from my side: 111c5a7 finally fixed build issues of the additional nginx container image required for static assets of django.

I'll now wait for changes of @jdemaeyer regarding possible changes to the openAPI spec

@jdemaeyer
Copy link
Collaborator Author

@ubergesundheit I have added hack-ish solutions for adding 400 and 404 responses, and for marking read-only properties as required :) This is likely as far as we can go without putting too much effort in, OpenAPI support in Django Rest Framework is still very new...

@ubergesundheit
Copy link
Member

Thanks, I'll check it out 👍

@ubergesundheit
Copy link
Member

So IMO this should be ready review. Thanks @jdemaeyer for your additional changes.

@bCyberBasti @ThorbenJensen lets keep this open to review once more

@bCyberBasti This branch also contains changes to the ansible playbook (containing django secret keys env changes), so it is required to execute the playbook on the server.

After this has been merged, we need to re-enable the "Check if generated client is up to date". It depends on the master container image but this is not up to date yet..

@ubergesundheit ubergesundheit merged commit 2d08643 into master Dec 31, 2020
@ubergesundheit ubergesundheit deleted the rewrite-backend-in-django branch December 31, 2020 16:20
ubergesundheit added a commit that referenced this pull request Jan 12, 2021
* Rewrite backend with Django

* Black-ify Django backend

* Fix missing backend dependency

* Remove trailing slahes from backend

* Add date, location, and organizer filters to events API

* Update backend README

* Black-ify all the things, again...

* Add Django admin for events

* Update backend flake8 config for new Django setup

* Fix help text for formatted_description field

* Update frontend API client

* Fix backend Docker container binding

* Use black to make files less readable

* Preserve event edits across crawls

* add neccessary env stuff for django to infrastructure

* prepare backend for deployment & add nginx static container

* try fix ci

* maybe - instead of _

* try out build-push-action@v2 for local caching

* use dest instead of src for cache-to

* try to address container source tag with digest

* try out local registry

* initialize buildx with network=host

* fix the yaml

* use correct build-arg

* Mark all readOnly properties as required in OpenAPI Spec

* Add hardcoded 400 and 404 response schemas to OpenAPI spec

* add updated generated-client

* change useGetEvents hook to accomodate changed backend

* attempt to fix client check in ci

* debug client check in ci

* disable client check for now

* add ssh-key gerald

* shave a step from backend dockerfile

* change migration command & rename static service in deployment compose file

* add some deployment docs

* change staging commands to use django

* add ansible.cfg

Co-authored-by: Gerald Pape <[email protected]>
Co-authored-by: ubergesundheit <[email protected]>
Co-authored-by: Gerald Pape <[email protected]>
ubergesundheit added a commit that referenced this pull request Jan 19, 2021
* Remove DateArrow

* add calendar popup

* super little style tweaks

* Bump @types/react from 16.9.49 to 17.0.0 in /frontend (#291)

* Bump typescript from 4.0.3 to 4.1.2 in /frontend (#290)

* Bump eslint-plugin-react from 7.21.3 to 7.21.5 in /frontend (#289)

* deprecate set-env (#292)

* Bump eslint-plugin-jsx-a11y from 6.4.0 to 6.4.1 in /frontend (#295)

* Bump @popperjs/core from 2.5.3 to 2.5.4 in /frontend (#294)

* Bump @testing-library/jest-dom from 5.11.4 to 5.11.6 in /frontend (#293)

* Bump @testing-library/react from 11.0.4 to 11.2.2 in /frontend (#296)

* Bump @testing-library/user-event from 12.2.2 to 12.5.0 in /frontend (#298)

* Bump @types/jest from 26.0.15 to 26.0.16 in /frontend (#297)

* Bump @types/node from 14.14.6 to 14.14.13 in /frontend (#302)

* Bump actions/setup-node from v2.1.2 to v2.1.3 (#299)

* Bump @types/react-dom from 16.9.8 to 17.0.0 in /frontend (#301)

* Bump actions/setup-node from v2.1.3 to v2.1.4 (#305)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from v2.1.3 to v2.1.4.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v2.1.3...c46424e)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix react-router scrolling bug (#306)

* fix react-router scrolling bug

* fixed styling

* fixed styling

* Rewrite backend with Django (#268)

* Rewrite backend with Django

* Black-ify Django backend

* Fix missing backend dependency

* Remove trailing slahes from backend

* Add date, location, and organizer filters to events API

* Update backend README

* Black-ify all the things, again...

* Add Django admin for events

* Update backend flake8 config for new Django setup

* Fix help text for formatted_description field

* Update frontend API client

* Fix backend Docker container binding

* Use black to make files less readable

* Preserve event edits across crawls

* add neccessary env stuff for django to infrastructure

* prepare backend for deployment & add nginx static container

* try fix ci

* maybe - instead of _

* try out build-push-action@v2 for local caching

* use dest instead of src for cache-to

* try to address container source tag with digest

* try out local registry

* initialize buildx with network=host

* fix the yaml

* use correct build-arg

* Mark all readOnly properties as required in OpenAPI Spec

* Add hardcoded 400 and 404 response schemas to OpenAPI spec

* add updated generated-client

* change useGetEvents hook to accomodate changed backend

* attempt to fix client check in ci

* debug client check in ci

* disable client check for now

* add ssh-key gerald

* shave a step from backend dockerfile

* change migration command & rename static service in deployment compose file

* add some deployment docs

* change staging commands to use django

* add ansible.cfg

Co-authored-by: Gerald Pape <[email protected]>
Co-authored-by: ubergesundheit <[email protected]>
Co-authored-by: Gerald Pape <[email protected]>

* Django staging fixes (#312)

* fix env for api container

* enable cors for django

* try reenabling api client validation

* Django staging fixes 2 (#313)

* rename traefik router of api-static

* document admin user creation

* Django staging fixes 3 (#314)

* fix DJANG_DEBUG env default

* change production deployment command in ci

* set SESSION_COOKIE_SECURE and CSRF_COOKIE_SECURE to True

* Upgrade deps (2020-12-31) (#315)

* upgrade deps

* revert dev stuff

* changes how date paths are generated (#311)

* changes how date paths are generated

* fixes import statement

Co-authored-by: Gerald Pape <[email protected]>

* scrapy raises exception on error status code (#308)

* scrapy now raises an exception when the request returns an error status code

* fixes performance issue

* fixed styling

* refactor

* fixes performance issue

* frontend deps upgrade 2020-01-04 (#322)

* Bump ky from 0.24.0 to 0.25.1 in /frontend (#317)

Bumps [ky](https://github.com/sindresorhus/ky) from 0.24.0 to 0.25.1.
- [Release notes](https://github.com/sindresorhus/ky/releases)
- [Commits](sindresorhus/ky@v0.24.0...v0.25.1)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump csp-html-webpack-plugin from 4.0.0 to 5.0.1 in /frontend (#319)

Bumps [csp-html-webpack-plugin](https://github.com/slackhq/csp-html-webpack-plugin) from 4.0.0 to 5.0.1.
- [Release notes](https://github.com/slackhq/csp-html-webpack-plugin/releases)
- [Commits](slackhq/csp-html-webpack-plugin@v4.0.0...5.0.1)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* remove staticmethod (#323)

* Bump lxml from 4.5.2 to 4.6.2 in /backend (#324)

Bumps [lxml](https://github.com/lxml/lxml) from 4.5.2 to 4.6.2.
- [Release notes](https://github.com/lxml/lxml/releases)
- [Changelog](https://github.com/lxml/lxml/blob/master/CHANGES.txt)
- [Commits](lxml/lxml@lxml-4.5.2...lxml-4.6.2)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump @types/jest from 26.0.19 to 26.0.20 in /frontend (#328)

* Bump eslint-plugin-prettier from 3.3.0 to 3.3.1 in /frontend (#327)

* Bump ky from 0.25.1 to 0.26.0 in /frontend (#325)

* Bump @types/node from 14.14.19 to 14.14.20 in /frontend (#326)

* Bump @testing-library/react from 11.2.2 to 11.2.3 in /frontend (#329)

* make calendar only full width for xs screens

* add close button to calendar

* align calendarbutton

* fiddle with spacings a little

* change size of date button in calendar again to keep it circular

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Benedikt Eble <[email protected]>
Co-authored-by: Jakob de Maeyer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add /sources endpoint
2 participants