Added server_args to AbstractApp #1173
Conversation
|
Whats the status on this commit? |
|
@tahalukmanji can you explain why you propose this change? What problem are you trying to solve? |
|
We use connexion for REST API and the web endpoint. We are having difficulties sharing common code between pytest runs and live production server runs. The primary issue is that the path to the templates folder is relative, and pytest runs change working directory paths. We want to have an ability to pass the path to the templates folder through the connexion api. That's what my code changes enable. So I can pass in the templates folder path from the connexion constructor. I don't think the change is very harmful overall, I enabled the passthrough because having the ability to control the flask object is important according to the users taste is necessary. If you think that interaction with the flask object has to be instantiated a certain way, then we should look at other ways of passing in the template folder Anyways, let me know whether this pull request will get merged or not. We can then plan accordingly. We have another idea in my mind but its an ugly ducktaping solution and we really don't want to implement it. |
connexion/apps/abstract.py
Outdated
| @@ -10,7 +10,7 @@ | |||
|
|
|||
| class AbstractApp(metaclass=abc.ABCMeta): | |||
| def __init__(self, import_name, api_cls, port=None, specification_dir='', | |||
| host=None, server=None, arguments=None, auth_all_paths=False, debug=None, | |||
| host=None, server=None, server_args=dict(), arguments=None, auth_all_paths=False, debug=None, | |||
There was a problem hiding this comment.
This should be server_args=None
https://docs.quantifiedcode.com/python-anti-patterns/correctness/mutable_default_value_as_argument.html
There was a problem hiding this comment.
I shall submit a new change...
|
Hello team, I submitted another commit based on feedback. Can someone please review this and get it merged? Or suggest other comments to get it merged? |
|
What have the gatekeepers decided? |
|
Sorry for taking so long to review the PR. Can you also add |
* server_args are then passed to Flask aio_https constructors * Updated documentation to reflect the change
Hi @jmcs , I have made modifications based on your comment. Please review and let me know if any further changes are required? |
Hello @JCMS and other reviewers, is there anything else to be done? Can we proceed to merge this PR into your master branch? |
|
Hello gatekeepers, what's the status of this review? |
|
👍 |
|
Thank you @hjacobs and others. Please let me know when this will be available in pypi mirrors. |
Fixes # .
Changes proposed in this pull request:
Changed AbstractApp constructor so to accept Flask server args
Changed create_app to pass Flask server args to Flask constructor