Added custom flask app wrapper to consume foca as config
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.
- The following resulted in attribute error from
mypy
foca_conf = current_app.config.foca
- But this one did not
foca_conf = current_app.app.config.foca
Further, to fix the same the following was used.
foca_conf = getattr(current_app.config, 'foca')
In addition, following are still being ignored from mypy
- celery_app.py
class ContextTask(celery.Task): # type: ignore
- auth.py
try:
claims = jwt.decode(
jwt=token,
verify=True,
key=key, # type: ignore[arg-type]
algorithms=algorithms,
audience=audience,
options=validation_options,
)
request.headers = \
ImmutableMultiDict(req_headers) # type: ignore[assignment]
- register_access_control.py
app.add_api(
specification=spec.path[0], # type: ignore[index]
**spec.dict().get("connexion", {}),
)
Fixes #144
Type of change
Please delete options that are not relevant.
- [x] Bug fix (non-breaking change which fixes an issue)
- [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
Checklist:
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have updated the documentation (or documentation does not need to be updated)
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] I have not reduced the existing code coverage
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
0d21a08) 100.00% compared to head (70bae41) 100.00%.
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
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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 current_app.config.foca.some_param or similar?
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
current_app.config.foca.some_paramor similar?
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 flask itself which would be a major switch.
The other way might be to create a wrapper context variable/global variable and utilise it across but that would imply changes across all foca clients.
To answer no it would not solve the above issue and the only workaround for FOCA built apps is to use getattr method for access foca in order to not fall for mypy failures.
I put this review on hold until we have discussed this on Slack given the approach I suggested.