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

Refactor main.py #949

Closed
rviscomi opened this issue Jul 3, 2020 · 10 comments · Fixed by #1176
Closed

Refactor main.py #949

rviscomi opened this issue Jul 3, 2020 · 10 comments · Fixed by #1176
Labels
development Building the Almanac tech stack good first issue Good for newcomers

Comments

@rviscomi
Copy link
Member

rviscomi commented Jul 3, 2020

The main.py server code has grown in size and complexity and it may make maintenance easier to refactor it into smaller, more discrete files.

Some things that can be grouped:

  • Flask subclass
  • template functions
  • static helper functions
  • chapter-specific routes
  • error routes
  • static file routes
  • ebook functions

@bazzadp WDYT?

@rviscomi rviscomi added the development Building the Almanac tech stack label Jul 3, 2020
@rviscomi rviscomi added this to the 2020 Platform Development milestone Jul 3, 2020
@tunetheweb
Copy link
Member

Yeah could probably do with a bit of clean up alright. Are you suggesting 7 files? Not sure I'd go that far.

One other thing I'd like to do is remove these out of the top level directory into a src/python directory or similar.

@rviscomi
Copy link
Member Author

rviscomi commented Jul 3, 2020

+1 for a subdirectory to organize the code!

Not suggesting each grouping has its own file.

@ibnesayeed
Copy link
Contributor

Are we only talking about reorganizing the code or something beyond that? I have been considering Responder to replace Flask in some of my projects going forward.

@tunetheweb
Copy link
Member

tunetheweb commented Jul 3, 2020

Sky's the limit! So whatever we think is best.

Why's it better than Flask? Not averse to moving, but always like a good reason for moving away something that's working fine.

Also, as per some goals I suggested for this year if we are doing to take the disruption of moving then I'd prefer to consolidate the tech stack and maybe move more to JavaScript with Node and EJS, rather than a sideways move to another Python based server. Unless of course there's a good reason for it!

@ibnesayeed
Copy link
Contributor

Why's it better than Flask?

I am not too excited about unnecessarily switching stack, but I mentioned Responder here because it has an elegant API, has built-in testing support with the popular Requests Python HTTP library (the two projects are steered by the same person), RESTful resource classes, API documentation, and an over-all more modern feel to its design is what feels appealing (look at their README).

@ibnesayeed
Copy link
Contributor

Also, as per some goals I suggested for this year if we are doing to take the disruption of moving then I'd prefer to consolidate the tech stack and maybe move more to JavaScript with Node and EJS, rather than a sideways move to another Python based server. Unless of course there's a good reason for it!

I looked at the stated goals and to be honest I would prefer going to the static site route. A lot can be done using data files and Jekyll to cover some gaps. Writing Ruby plugin for Jekyll can help cover some missing parts. After the inception of GH Actions, we do not have to limit ourselves to the Jekyll plugins whitelisted by GitHub Pages.

@rviscomi
Copy link
Member Author

rviscomi commented Jul 3, 2020

Are we only talking about reorganizing the code or something beyond that? I have been considering Responder to replace Flask in some of my projects going forward.

For the purposes of this issue, it's narrowly scoped to reorganizing the server code.

I think switching platforms is a more serious discussion that needs to be designed/debated during the off-months to ensure that it's not in the critical path of the launch. Let's not consider it blocking for 2020.

@tunetheweb
Copy link
Member

@HTTPArchive/developers anyone with good Python skills fancy tackling this one?

@SaptakS
Copy link
Collaborator

SaptakS commented Aug 4, 2020

I can give it a look over the weekend.

Super-Fly added a commit to Super-Fly/almanac.httparchive.org that referenced this issue Aug 7, 2020
…n code moved to app/ directory, helper functions in separate file, routes in separate files, init file for creating and configuring the Flask app
@Super-Fly
Copy link
Contributor

Hey, I created a PR for that, can you take a look and see if it looks ok?
There a couple of things that can be improved but my focus in that PR was just to clean up main.py for now.

tunetheweb added a commit that referenced this issue Aug 9, 2020
* Refactor main.py #949 - code split in few files, all Python code moved to app/ directory, helper functions in separate file, routes in separate files, init file for creating and configuring the Flask app

* Moved get_chapter_nextprev function to helpers

* Move config.py to app folder

* Add new app/config.py file (forgot to include)

* Fix linting errors

* Just a few naming bikesheds :)

* Created errors.py, move everything from main.py, moved Unit tests to sub directory, made talisman and app global for routes and errors

* Add Flake8

* Flake8 fixes

* Linting errors

* Added me as contributor for 2020

* More linting fixes

* Linting fixes

* Linting fixes

* Linting fixes

* Linting fixes

* Linting fixes

* Remove GitHub Action changes

* Remove Github workflow changes

Co-authored-by: Barry <barry@tunetheweb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Building the Almanac tech stack good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants