search

HackSoc / hacksoc.org

HackSoc website
https://www.hacksoc.org
8 stars 17 forks source link

Python backend replacement #165

Closed LukeMoll closed 3 years ago

LukeMoll commented 3 years ago

The current (nodejs) build system is poorly written, partially documented, and not very maintainable. In order to address these going forward, the backend is rewritten in Python using Flask and the much more powerful Jinja templating engine. This, along with better documentation, should allow new features to be added easily when needed.

Either that or I didn't want to graduate while being the only person who comprehends the mess of javascript that's building the site currently ;)

TODO:

Feedback welcomed even during draft status, do note TODO list of known missing features.

Breaking changes

Generated web root (from flask freeze) is now at build/, not html/ as previously (this could be configured however).

Issues

(Closes #91 fwiw)

LukeMoll commented 3 years ago

@FromAnkyra That's valid yeah, it's explained a little more in the docs ( https://github.com/HackSoc/hacksoc.org/blob/2021-09_python-rewrite/docs/adding_features_python.md#modifying-freeze) but some of that explanation should be on the front page too. On Sun, 19 Sep 2021, 10:39 am fromankyra, @.***> wrote:

@.**** commented on this pull request.

In README.md https://github.com/HackSoc/hacksoc.org/pull/165#discussion_r711715762:

-### Server READMEs -Written in [Markdown]. Like regular pages, has a YAML header:

    • hostname: lowercase hostname of the server
    • fqdn: full domain of the server, typically <hostname>.hacksoc.org
    • name: purpose/subtitle of the server (eg "shell server") +### Freezing to HTML

It isn't really clear to me what this means? Maybe there's more detail in the proper docs, and this may be my "never written a proper website"-ness showing, but I don't know what freezing to html is, nor whether I need to do it if I want to change the website. A quick google search for "freezing html" gives me a bunch of different things so isn't especially helpful.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/HackSoc/hacksoc.org/pull/165#pullrequestreview-758130722, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2N5UZZBQXA6QGQA5DQL73UCWVWTANCNFSM5DZNVTXQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

FromAnkyra commented 3 years ago

@LukeMoll basically imo either it should have a sentence explaining what it is in the first readme (or even a link to that part of the docs tbh), or it shouldn't get mentioned there at all. I'm currently going through the docs as if I were completely new to the codebase and only want to modify the website in one specific way, and in that use case, seeing the freeze thing just confuses me

LukeMoll commented 3 years ago

@JMAlego I think MyPy is going to be a non-priority with this PR; Flask and its components seem to have questionable type support and static type checking is of limited use in this instance, there are few layers of code before data passes through a layer that makes it impossible to introspect types (jinja templating for example). If someone would like to add it later then I'm not against that, but I won't be supporting it in this PR.

LukeMoll commented 3 years ago

@JMAlego your Windows-related bug should be fixed in 2fc46e8, could you confirm? Any additional testing/trying-to-break-it on Windows is thoroughly appreciated as it's not cases I run into regularly; that's another tick in the "cross-platform unit testing" column

LukeMoll commented 3 years ago

Tests have been given class but not module docstrings, since each test module contains exactly one class.

LukeMoll commented 3 years ago

@sersorrel I've gotten it building on Linux and Windows now. Infra Things will need to be done, notably:

JMAlego commented 3 years ago

I can check the building of it on my machine again tomorrow evening if you want?

LukeMoll commented 3 years ago

If you could that'd be great, with flask freeze be sure to check the build/ folder that everything you'd expect would be there

On Mon, 20 Sep 2021, 11:55 pm Jacob Allen, @.***> wrote:

I can check the building of it on my machine again tomorrow evening if you want?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HackSoc/hacksoc.org/pull/165#issuecomment-923412880, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2N5U3Z2W4EM5WQ7SHNT3TUC63UJANCNFSM5DZNVTXQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

LukeMoll commented 3 years ago

Okay I think I've addressed everything fed back

LukeMoll commented 3 years ago

@JMAlego Encodings fixed in 6baa108