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

Add session variable to DBSession init #17

Closed
wants to merge 1 commit into from
Closed

Add session variable to DBSession init #17

wants to merge 1 commit into from

Conversation

teamhide
Copy link
Contributor

@teamhide teamhide commented Jul 23, 2020

While using db.session on pycharm, it notify Unresolved attribute reference 'session' for class 'DBSession' errors.

To avoid it, added self.session to DBSession.

@codecov-commenter
Copy link

Codecov Report

Merging #17 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #17   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           60        61    +1     
=========================================
+ Hits            60        61    +1     
Impacted Files Coverage Δ
fastapi_sqlalchemy/middleware.py 100.00% <100.00%> (ø)

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 b46b9e6...4df5289. Read the comment docs.

@mfreeborn
Copy link
Owner

Thanks for the PR. I don't use Pycharm myself. Happy to resolve this problem. Could I ask you to try one alternative and tell me if this fixes it? Instead of adding self.session = None, what happens if you change line 82 of middleware.py from db: DBSession = DBSession to db: DBSessionMeta = DBSession? Does that fix it or not?

@teamhide
Copy link
Contributor Author

teamhide commented Jul 27, 2020

Works perfectly. I will re-pr for this issue.

@teamhide teamhide closed this Jul 27, 2020
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.

None yet

3 participants