-
Notifications
You must be signed in to change notification settings - Fork 12
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
Added custom flask app wrapper to consume foca as config #200
base: dev
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #200 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 21 +1
Lines 975 995 +20
=========================================
+ Hits 975 995 +20 ☔ View full report in Codecov by Sentry. |
Before reviewing: Did you check whether this solution solves the mypy issues in apps being built with FOCA when they try to access FOCA config params via |
I did check on the possibilities for doing this but eventually came to a conclusion that for using this we would need to alter the typed configurations in To answer no it would not solve the above issue and the only workaround for FOCA built apps is to use |
I put this review on hold until we have discussed this on Slack given the approach I suggested. |
Description
A number of mypy errors were coming in since foca was not included as part of the flask config but was being consumed using the current app context and otherwise too.
To fix the same, I created a custom wrapper to create a flask app with an underlying config attribute for foca. Thereby removing the mypy errors.
However it was observed that while consuming the attribute from current_app context, still mypy errors were coming.
mypy
Further, to fix the same the following was used.
In addition, following are still being ignored from
mypy
Fixes #144
Type of change
Please delete options that are not relevant.
Checklist: