safely convert ini parameters to bool, int, list and dict #104
Conversation
| if value.lower() in ("true", "false"): | ||
| return bool(value) |
There was a problem hiding this comment.
For this, I suggest using pyramid.settings.asbool to consider the full range of supported "bool-like" values.
There was a problem hiding this comment.
I did consider this, however, this is trying to replicate what you might use in celeryconfig.py, but with values you would find in the ini file.
For example in celeryconfig.py you might write
task_acks_late = True
so in the ini you would need to write
task_acks_late = true
Pyramid asbool will also convert integer and other values to a boolean which may or may not work similarly to setting them celeryconfig.py
There was a problem hiding this comment.
Good point about the integer. Better to stay safe with the explicit bool-like strings. 👍
| @@ -106,7 +118,7 @@ def read_configuration(self, fail_silently=True): | |||
| config_dict = {} | |||
|
|
|||
| for key, value in self.parser.items('celery'): | |||
| config_dict[key] = value | |||
| config_dict[key] = safe_conversion(value) | |||
There was a problem hiding this comment.
Might need to consider applying this to other structures below?
For example, a lot of Task result backend settings tend to have very nested dict definitions.
|
The latest push adds json5 as safely converted value.. json5 is a human friendly json parser (accepts both ' and " ) and other things. I could also use a python AST parser if you prefer |
pyramid_celery/loaders.py
Outdated
| try: | ||
| newval= json5.loads(value) | ||
| #print("JSON DECODED", type(newval), newval) | ||
| return newval | ||
| except JSONDecodeError: | ||
| pass |
There was a problem hiding this comment.
This should probably try using json.load as fallback if pyjson5 is not installed.
The imports should also handle missing pyjson5 from installed packages.
I don't think it is necessary for pyramid_celery to inject this new dependency.
|
I feel literal_eval is better solution.
|
|
I agree, |
kgk
changed the title
|
Just a ping to check if the patch is accepted |
pyramid_celery/loaders.py
Outdated
| try: | ||
| if float(value).is_integer(): | ||
| return int(value) | ||
| return float(value) | ||
| except ValueError: | ||
| pass |
There was a problem hiding this comment.
I don't think this is needed anymore since literal_eval should handle it as well.
|
@kgk |
|
@fmigneault please add to your list |
This will convert ini paramerts to bool or int if they can be.
Allows for max_tasks_per_child to be set as well as others for celery workers
fixes : #103