Improve openfisca serve documentation to make parameter name change inconsistency more obvious #593
Conversation
Reinforce awarness about gunicorn parameter usage in config file and cmd line
|
Seems good for me :) @sandcha ? |
There was a problem hiding this comment.
Thank you @jvalduvieco for these improvements!
There is only one change to be done on the config.py and minor comments.
|
|
||
| .. code-block:: shell | ||
|
|
||
| gunicorn --help |
doc/source/openfisca_serve.rst
Outdated
| @@ -42,14 +48,16 @@ Serving reforms | |||
|
|
|||
| Using a configuration file | |||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ | |||
| You can setup `openfisca serve` using a configuration file. Be careful as parameters with a '-' in their name change to an '_' when used from the config file. | |||
There was a problem hiding this comment.
Use same style as above by using double backticks on command name?
And, what do you think about describing the name changes as follows? :
You can setup ``openfisca serve`` using a configuration file. Be careful as it is a Python file using Python naming rules: parameters with a ``-`` in their name on command line change to an ``_`` when used from the config file.
doc/source/openfisca_serve.rst
Outdated
|
|
||
| **config.py:** | ||
|
|
||
| .. code-block:: py | ||
|
|
||
| port = 4000 | ||
| workers = 4 | ||
| country-package = 'openfisca_france' | ||
| bind = 0.0.0.0:4000 | ||
| country_package = 'openfisca_france' |
doc/source/openfisca_serve.rst
Outdated
|
|
||
| **config.py:** | ||
|
|
||
| .. code-block:: py | ||
|
|
||
| port = 4000 | ||
| workers = 4 | ||
| country-package = 'openfisca_france' | ||
| bind = 0.0.0.0:4000 |
There was a problem hiding this comment.
Update to bind = '0.0.0.0:' + str(port) as bind takes a string value and port variable is defined above?
config.pyis an extension of gunicorn configuration file. It manages openfisca specific configuration likeport.portvalue will also updatebindvalue (openfisca configuration has higher priority here than gunicorn's configuration).
|
It seems that all the comments have been taken into account. Is this PR good to merge @sandcha ? |
Breaking changes
None
New features
Deprecations
Technical changes