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

Changed the the entity_type kwarg to table_name #111 and register_entity now throws exception if user hasn't passed table_name #91 #112

Merged
merged 14 commits into from
Dec 6, 2019

Conversation

kousikmitra
Copy link
Contributor

@kousikmitra kousikmitra commented Dec 5, 2019

#111

  1. Changed the the entity_type kwarg to table_name
  2. Added Deprecation warning for the entity_type kwargs

#91

  1. Added test function to test whether register_entity throws an exception on not passing the table_name key
  2. Added code to throw KeyError exception in register_entity function of FlaskJWTRouter class if user doesn't pass a table_name

@codecov
Copy link

codecov bot commented Dec 5, 2019

Codecov Report

Merging #112 into master will decrease coverage by 0.87%.
The diff coverage is 80.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
- Coverage   88.93%   88.05%   -0.88%     
==========================================
  Files           7        7              
  Lines         262      268       +6     
  Branches       29       31       +2     
==========================================
+ Hits          233      236       +3     
- Misses         23       25       +2     
- Partials        6        7       +1
Impacted Files Coverage Δ
flask_jwt_router/_jwt_routes.py 100% <ø> (ø) ⬆️
flask_jwt_router/_authentication.py 90.9% <100%> (ø) ⬆️
flask_jwt_router/_entity.py 76.78% <100%> (ø) ⬆️
flask_jwt_router/_jwt_router.py 89.47% <58.33%> (-4.65%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e3b04f...2a7519c. Read the comment docs.

@kousikmitra kousikmitra changed the title register_entity now throw exception if user hasn't passed a entity_type #91 Changed the the entity_type kwarg to table_name #111 and register_entity now throws exception if user hasn't passed table_name #91 Dec 6, 2019
@joegasewicz
Copy link
Owner

Nice work @kousikmitra, thank you.

@joegasewicz
Copy link
Owner

The API is now looking much better already, thanks

@kousikmitra
Copy link
Contributor Author

@joegasewicz Thanks. Can you please take a look what is the issue with Codacy?

@joegasewicz
Copy link
Owner

joegasewicz commented Dec 6, 2019

@joegasewicz Thanks. Can you please take a look what is the issue with Codacy?

Screenshot 2019-12-06 at 12 06 39

@joegasewicz
Copy link
Owner

@kousikmitra can you run make html to update the docs, also can you update the README.md contributors instructions with this info also, thank you.

@kousikmitra
Copy link
Contributor Author

kousikmitra commented Dec 6, 2019

@kousikmitra can you run make html to update the docs, also can you update the README.md contributors instructions with this info also, thank you.

make html is giving this error.(I am running on windows)

process_begin: CreateProcess(NULL, sphinx-build -M html source build, ...) failed.
make (e=2): The system cannot find the file specified.
make: *** [Makefile:20: html] Error 2

Do I need any extra modules for this?

@kousikmitra
Copy link
Contributor Author

@joegasewicz why is it showing get_entity_from_ext as invalid syntax?

@joegasewicz
Copy link
Owner

joegasewicz commented Dec 6, 2019

@kousikmitra can you run make html to update the docs, also can you update the README.md contributors instructions with this info also, thank you.

make html is giving this error.(I am running on windows)

process_begin: CreateProcess(NULL, sphinx-build -M html source build, ...) failed.
make (e=2): The system cannot find the file specified.
make: *** [Makefile:20: html] Error 2

Do I need any extra modules for this?

can you install Sphinx & ill update the Pipfile in my PR, thanks

@joegasewicz
Copy link
Owner

@joegasewicz why is it showing get_entity_from_ext as invalid syntax?

I have added your email to our codacy organization.

@kousikmitra
Copy link
Contributor Author

kousikmitra commented Dec 6, 2019

@joegasewicz why is it showing get_entity_from_ext as invalid syntax?

I have added your email to our codacy organization.

How can I access this?

Edit: Can you please check what is the problem with CHANGELOG.md file?

@joegasewicz
Copy link
Owner

@joegasewicz why is it showing get_entity_from_ext as invalid syntax?

I have added your email to our codacy organization.

How can I access this?

https://app.codacy.com/manual/joegasewicz/Flask-JWT-Router/dashboard?bid=15313481

@joegasewicz
Copy link
Owner

joegasewicz commented Dec 6, 2019

Just this one now to fix, i ignored the inline docs on the abstract methods

Screenshot 2019-12-06 at 13 07 16

@joegasewicz
Copy link
Owner

@kousikmitra let mew know when you have had enough and ill merge your PR in lol

@joegasewicz joegasewicz closed this Dec 6, 2019
@joegasewicz joegasewicz reopened this Dec 6, 2019
@joegasewicz
Copy link
Owner

Sorry meant to comment and closed by mistake.

@joegasewicz
Copy link
Owner

@kousikmitra let me know when you're happy with the PR & ill merge in, thanks

@kousikmitra
Copy link
Contributor Author

@kousikmitra let me know when you're happy with the PR & ill merge in, thanks

it's was checking fora tab and I was giving a space 💢 What to include in the README file?

@joegasewicz
Copy link
Owner

@kousikmitra let me know when you're happy with the PR & ill merge in, thanks

it's was checking fora tab and I was giving a space 💢 What to include in the README file?

Please add:

To update docs run:

make html

@joegasewicz joegasewicz merged commit 60abd43 into joegasewicz:master Dec 6, 2019
@joegasewicz
Copy link
Owner

Thanks @kousikmitra, great work!

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