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

Refactoring backend #627

Closed
djmgit opened this issue Apr 15, 2018 · 26 comments
Closed

Refactoring backend #627

djmgit opened this issue Apr 15, 2018 · 26 comments

Comments

@djmgit
Copy link
Member

djmgit commented Apr 15, 2018

Presently all the route handlers and app config are presently in a single file : backend/app/main.py. As a result, main.py is already quite big and as more route handlers and other methods will be added, the code will become really hard to maintain owing to the code size. So I guess it is high time now we modularise the APIs.
We can use flask blueprint to distribute the API handlers among different modules. In future when we will be adding models (for database) we can add specific models to corresponding modules rather than keeping all the models in the same place.
Also we can maintain a separate file for config params and then initialise app.config from that file.
This will make the code easy to maintain, also we will be able to reuse stuff.

@djmgit
Copy link
Member Author

djmgit commented Apr 15, 2018

@vaibhavsingh97 @ParthS007 @gabru-md what are your views?

@djmgit
Copy link
Member Author

djmgit commented Apr 15, 2018

This article provides some nice workflow for modularizing flask app:
https://www.digitalocean.com/community/tutorials/how-to-structure-large-flask-applications

@gabru-md
Copy link
Member

@djmgit I have a proposed plan for the same.
We need to substructure our codebase for ease of understanding and to make it efficient.
I completely agree with your point of distributing the API in different files.

We will need to completely re write the codebase in order to ensure proper functioning and flow.
So that we can maybe improvise the current codescheme. We even need to integrate a database service for maintaining the badges produced. Can we discuss it here ?

@djmgit
Copy link
Member Author

djmgit commented Apr 15, 2018

@gabru-md yes sure
yes we definitely require a database service soon.

@vaibhavsingh97
Copy link
Member

@djmgit Yes, I agree, I had added the same in my proposal too. We can definitely do this, and make this app modularize. For backend Service we can use PostgreSQL or any other NoSQL database.

@vaibhavsingh97
Copy link
Member

How about making a new branch (The branch will be temporary) and start working on modularizing? Also, I had opened the issue (#459) long time ago. We can start by moving step by step like extracting configurations and adding in one file etc.
@djmgit Your views?

@S2606
Copy link
Member

S2606 commented Apr 15, 2018

@djmgit You mean creating routers and controllers??

@S2606
Copy link
Member

S2606 commented Apr 15, 2018

routers where the endpoints shall be written and controllers where the specific control for a particular endpoint be there.

@S2606
Copy link
Member

S2606 commented Apr 15, 2018

Also I was thinking maybe we can make badgeyay more asynchronous using libs like tornado.Please give me your feedback.

@djmgit
Copy link
Member Author

djmgit commented Apr 15, 2018

@vaibhavsingh97 Since at this stage, the size of main.py is not that big, also we do not have models and serializers to deal with right now, do we really need to create a separate branch? Just asking :) I have no problem with creating one.

@ParthS007
Copy link
Member

ParthS007 commented Apr 15, 2018

@djmgit I totally agree with you and I have proposed somewhat similar to it.
We can proceed it in this way

  • Modularise the API
  • Also, modification needed in frontend as generating csv part is not working.
  • We need to discuss about the Database we are going to use.

What's your view @djmgit ?
Please correct me if I am wrong somewhere.
I don't think we need a separate branch for this work.

@djmgit
Copy link
Member Author

djmgit commented Apr 16, 2018

@ParthS007 we definitely need to rewrite the frontend, as presently the entire frontend code in written in two huge .hbs files. We need to use ember components which is the desired development flow with ember I suppose. We can discuss about frontend in a separate issue.
Yeah for database as @vaibhavsingh97 has already pointed out, we can either use PostgreSQL along with some ORM like SQLAlchemy or a noSql db like mongo. Ofcourse we require more discussion on this.

@gabru-md
Copy link
Member

@djmgit I don't think that we need to create a new branch as in future it may cause conflicts.
And since the codebase is not too big so we can stick to development branch in order to develop more into badgeyay.

@djmgit
Copy link
Member Author

djmgit commented Apr 17, 2018

@gabru-md @vaibhavsingh97 @ParthS007 once this issue is done, that is the backend codebase is refactored, we can go forward with the other important stuff deciding a database, creating an auth system, deciding db schemas required for various APIs etc.
I will be creating a similar to issue for frontend too so that we can keep the backend and frontend at sync almost all the time :)

@ParthS007
Copy link
Member

ok @djmgit 👍

@vaibhavsingh97
Copy link
Member

vaibhavsingh97 commented Apr 17, 2018

ok @djmgit , One Parent issue for both frontend or backend with tasks will be good.

@vaibhavsingh97
Copy link
Member

@djmgit Can I take the issue, I will open sub-issue and claim it. I will help in modularizing the current backend code

@djmgit
Copy link
Member Author

djmgit commented Apr 24, 2018

@vaibhavsingh97 I guess (from above comments) @gabru-md has also got some plans for this issue. It would be great if you both could collaborate on this, if possible :)

@vaibhavsingh97
Copy link
Member

vaibhavsingh97 commented Apr 24, 2018 via email

@gabru-md
Copy link
Member

gabru-md commented May 1, 2018

I would like to send a test PR for the same @djmgit .
It contains all the folders etc that we need to modularize the API. then we can work on making it better sideways so that we ensure the product stability and a refractored version being made sideways as well.
Can you please comment ?

gabru-md added a commit to gabru-md/badgeyay that referenced this issue May 1, 2018
Adds Blueprint Folder. This contains the new Refractored Code seperated into
folders like controllers, models and utils.
I will send multiple PRs to setup the Blueprint folder.
Meanwhile we can add functionality to the current codebase.

Cheers!
@ParthS007
Copy link
Member

Please go ahead @gabru-md , great 👍

@djmgit
Copy link
Member Author

djmgit commented May 1, 2018

@gabru-md please go ahead

@gabru-md
Copy link
Member

gabru-md commented May 1, 2018

So @ParthS007 and ME will work on modularising the backend along with refractoring it. By the time other changes are being made to the current API, we will make them stable into the Modularised API and get it working sideways 👍

gabru-md added a commit to gabru-md/badgeyay that referenced this issue May 1, 2018
Adds Blueprint Folder. This contains the new Refractored Code seperated into
folders like controllers, models and utils.
I will send multiple PRs to setup the Blueprint folder.
Meanwhile we can add functionality to the current codebase.

Cheers!
@open-event-bot open-event-bot bot added the has-PR label May 1, 2018
@yashLadha
Copy link
Member

Count me in too @gabru-md

@gabru-md
Copy link
Member

can be closed

djmgit pushed a commit that referenced this issue May 29, 2018
<!-- Add the issue number that is fixed by this PR (In the form Fixes #123) -->
Fixes #877 
#627 

#### Checklist

- [x] I have read the [Contribution & Best practices Guide](https://blog.fossasia.org/open-source-developer-guide-and-best-practices-at-fossasia) and my PR follows them.
- [x] My branch is up-to-date with the Upstream `development` branch.
- [x] I have added necessary documentation (if appropriate)

### Preview Link 

- **Replace XXX with your PR no**
- Link to live demo: http://pr-XXX-fossasia-badgeyay.surge.sh  

#### Changes proposed in this pull request:

- Changed Travis Check and Moved API to the backend.
@ParthS007
Copy link
Member

Closing this as we are done with it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants