Skip to content

Support Pagination and Column Blacklist#2

Merged
lockie merged 11 commits intolockie:masterfrom
twodayslate:master
Jan 24, 2020
Merged

Support Pagination and Column Blacklist#2
lockie merged 11 commits intolockie:masterfrom
twodayslate:master

Conversation

@twodayslate
Copy link
Contributor

@twodayslate twodayslate commented Jan 20, 2020

Used black for code formatting

For the sqla.py example you can try the following:

http://localhost:8888/api/posts/?limit=1
http://localhost:8888/api/posts/?limit=2&page=2

You'll see the server has a hard limit of 12 but you can request a smaller limit if desired.

Blacklisting is as simple as adding the column to the resource blacklist

@twodayslate twodayslate changed the title Support Pagination and Column Blacklist WIP: Support Pagination and Column Blacklist Jan 20, 2020
@twodayslate twodayslate changed the title WIP: Support Pagination and Column Blacklist Support Pagination and Column Blacklist Jan 20, 2020
Copy link
Owner

@lockie lockie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your efforts! There's some minor naming issues, an extra debug print and a need to modify DBAPI2Resource as you did with SQLAlchemyResource. Otherwise it looks good to me.

@twodayslate
Copy link
Contributor Author

Thank you for the indepth review.

@lockie
Copy link
Owner

lockie commented Jan 22, 2020

You're welcome!
There is minor issue with SQL limit/offset, and then I think your PR is ready to merge.
There are also some tests failing, but I'll fix those myself after.

@twodayslate
Copy link
Contributor Author

twodayslate commented Jan 22, 2020

Does that fix the issue(s)?

edit://

I changed limit and page to page[size] and page[number] respectfully to match JSON:API.

I have modified the meta as that is where the limit stuff is supposed to go based on JSON:API. I also added a links sections based on JSON:API pagination information as well.

Let me know if you want any other changes.

I added an auth.py example as I don't think it is obvious to new users that authentication can be added to the API (and perhaps should be).

I added a spec.py example which should help with the automatic documentation generation TODO you have. You can use sphinx-jsonschema to generate documentation. Anything else and I think this project would become very bloated. Should this be an official handle?

edit2://

StructuralWalker was bugging out for me with relationships for SQLAlchemy so I now use NoForeignKeyWalker and support relationships when rendering. Creating or updating not currently tested

@twodayslate twodayslate mentioned this pull request Jan 22, 2020
@twodayslate twodayslate changed the title Support Pagination and Column Blacklist WIP: Support Pagination and Column Blacklist Jan 22, 2020
@twodayslate twodayslate changed the title WIP: Support Pagination and Column Blacklist Support Pagination and Column Blacklist Jan 22, 2020
@lockie
Copy link
Owner

lockie commented Jan 23, 2020

Yeah, 964f725 fixes the limit/offset issue.
For other changes, you most definitely should've open new pull request with new branch, so that we can eat an elephant one bite at a time. Do you mind doing it and removing those new commits from this PR? Use git force push if you must.

@lockie lockie merged commit 65e6261 into lockie:master Jan 24, 2020
@lockie
Copy link
Owner

lockie commented Jan 24, 2020

Thanks again for your efforts!

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.

2 participants