new --reload-extra-file option #1527
Conversation
|
Seems okay to me! |
|
As far as I can tell, nothing will break if you don't validate that the files exist, but maybe it's the right idea so that users don't silently watch the wrong thing? |
garbas
force-pushed
the
reload-extra-files
branch
from
7b83935
to
7749c30
Compare
|
@tilgovi I usually follow the "fail early" rule and if somebody provides an option that points to a file which we should watch and that file does not exists it is very likely to be a typo. I would assume 99% of cases is like this. In the other 1%, you can always use I added the description for new option and squashed the work in one commit. As far as I can tell this looks ready to be merged, if not let me know what needs to change. |
gunicorn/config.py
Outdated
| @@ -379,6 +388,17 @@ def validate_list_string(val): | |||
| return [validate_string(v) for v in val] | |||
|
|
|||
|
|
|||
| def validate_list_of_existing_files(val): | |||
| if not val: | |||
There was a problem hiding this comment.
Maybe remove these few lines and replace with a call to validate_list_string?
gunicorn/config.py
Outdated
| @@ -368,6 +368,15 @@ def validate_string(val): | |||
| return val.strip() | |||
|
|
|||
|
|
|||
| def validate_file_exists(val): | |||
| val = validate_string(val) | |||
| if val is None: | |||
There was a problem hiding this comment.
I'm not sure we want this here. For the string options, I think it's there for when the default is None if the option wasn't provided at all. In the case of this option, the default is an empty list.
gunicorn/config.py
Outdated
| default = [] | ||
| desc = """\ | ||
| Extends --reload option to also watch and reload on additional files | ||
| (eg. templates, configurations, specifications, ...). |
There was a problem hiding this comment.
nit: "(e.g., templates, configurations, specifications, etc.)."
garbas
force-pushed
the
reload-extra-files
branch
from
7749c30
to
f736909
Compare
|
@tilgovi Thank you for the review. I fixed problems you mentioned in your comments. Does it look ok now? |
|
Thanks, @garbas. Merged. 👍 |
new --reload-extra-file option
I quickly added an option which is implemented but not exposed in the command line.
Would something like this be excepted? If yes I'll add the documentation for the command line option.
/cc @tilgovi (author of
gunicorn/reloader.py)