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 ODM2RestfulWebservices #11

Closed
lsetiawan opened this issue Aug 23, 2017 · 15 comments
Closed

Refactoring ODM2RestfulWebservices #11

lsetiawan opened this issue Aug 23, 2017 · 15 comments

Comments

@lsetiawan
Copy link
Member

Overview

I have explored @emiliom Demo for his Marchantaria database and view the results of each REST endpoint on AWS. Additionally, #8 shows my attempt to integrate the REST API into another Database, in this case @miguelcleon LCZO Database.

I think that the current code is doing what it supposed to and doing it pretty well, though it's not fully utilizing the ODM2PythonAPI. It is a similar case to WOFpy, ODM2RestfulWebServices is only using the models from ODM2PythonAPI.

Django ORM vs SQLAlchemy ORM

After researching and learning about Django ORM vs SQLAlchemy ORM, I found this article that summarize the differences. I think that sticking with using the power of SQLAlchemy ORM through ODM2PythonAPI is a good way to go for Read Only REST API, since we will be able to create a very complex queries with SQLAlchemy ORM compared to Django ORM. Additionally, SQLAlchemy is supported by many databases including MSSQL.

Code structure

Going through the code, I think that restructuring is needed. For example, many of the views are long and lots of the functions seems to be serializers rather than views. For example odm2rest/result_views.py#L776

Package updates

Obviously, a huge update is needed for the dependencies for ODM2RestfulWebServices. But, a lot of work will be needed to updates the swagger view for django rest framework. Here are a list of resources that will help start this effort.

Additionally, more integration with ODM2PythonAPI could be done to perform some queries that are already available.

@emiliom
Copy link
Member

emiliom commented Aug 24, 2017

Thanks for the assessment and summary, @lsetiawan!

One question:

I think that sticking with using the power of SQLAlchemy ORM through ODM2PythonAPI is a good way to go for Read Only REST API

While we don't have near-term plans to address data writing/push/updating via the REST API, it'd be useful to keep that future possibility in mind as we make architecture decisions. Can you say something about how SQLAlchemy ORM and Django ORM compare for that capability?

@emiliom
Copy link
Member

emiliom commented Aug 29, 2017

@lsetiawan, that's a really great start. I'll add to my TO DO's to refine your text a bit to improve its clarity, based on what you've told me.

@lsetiawan
Copy link
Member Author

@lsetiawan, that's a really great start. I'll add to my TO DO's to refine your text a bit to improve its clarity, based on what you've told me.

Great thanks!

While we don't have near-term plans to address data writing/push/updating via the REST API, it'd be useful to keep that future possibility in mind as we make architecture decisions. Can you say something about how SQLAlchemy ORM and Django ORM compare for that capability?

I will look into this. Not sure how they compare regarding writing/push/update via REST API.

@lsetiawan
Copy link
Member Author

I have made a proof of concept using Django, DjangoRestFramework, ODM2PythonAPI. See branch https://github.com/ODM2/ODM2RESTfulWebServices/tree/odm2rest_1_0/odm2rest.

screen shot 2017-09-19 at 15 16 38

@emiliom
Copy link
Member

emiliom commented Sep 19, 2017

Very cool, thanks! Looking forward to seeing it in more detail tomorrow.

@horsburgh
Copy link
Member

Let me know how this goes guys. We are sort of holding until we have a better idea of how to move forward. I would like to get Eli contributing to whichever direction we settle on.

@emiliom
Copy link
Member

emiliom commented Sep 20, 2017

I'll follow up very soon, @horsburgh. In the meantime, please reply on this thread to give us Eli's github handle and email address (you can send me his email address via email if you prefer)

@Elijahwalkerwest
Copy link
Contributor

Hey @emillion. This is eli, email is elijahwalkerwest@gmail.com

@lsetiawan
Copy link
Member Author

Even cooler proof of concept. I was able to figure out swagger also! It was actually super easy once it clicked in my head.

screen shot 2017-09-20 at 08 52 36

@emiliom
Copy link
Member

emiliom commented Sep 20, 2017

@lsetiawan: awesome! Looking forward to discussing it in a couple of hours.

@Elijahwalkerwest: Thanks for following up. I'll be reaching out later today.

@lsetiawan
Copy link
Member Author

@emiliom I have made a branch odm2rest_legacy which has the current master branch. I was thinking maybe it's time to make the new master to the current development? what do you think? Thanks!

@emiliom
Copy link
Member

emiliom commented Oct 23, 2017

I was thinking maybe it's time to make the new master to the current development? what do you think?

I agree. Go for it. But please add a brief statement about this at the start of the README doc, with a link to the legacy branch.

@lsetiawan
Copy link
Member Author

Code has been refractored as of Oct 23. Latest development code is now in master branch. The old API code is mentioned on the README doc with a link.

@emiliom
Copy link
Member

emiliom commented Nov 29, 2017

FYI, I just edited the README file (directly on the repo, no PR, sorry), mainly to create a new section addressing the "Legacy prototype version" more clearly and in a bit more details.

@lsetiawan
Copy link
Member Author

Sounds good. Thanks for letting me know. I like the more detailed version.

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

4 participants