Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

safely convert ini parameters to bool, int, list and dict #104

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
49 changes: 49 additions & 0 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
image: python:latest

variables:
PIP_CACHE_DIR: "$CI_PROJECT_DIR/.cache/pip"
#
PUBLIC_REGISTRY_PROJECT_ID: 32304092

cache:
paths:
- .cache/pip
- venv/

stages:
- check
- deploy

before_script:
- python -V # Print out python version for debugging

ruff:
stage: check
script:
- pip install ruff
- ruff check --output-format=gitlab --output-file gl-code-quality-report.json pyramid_celery
artifacts:
reports:
codequality: gl-code-quality-report.json

# test:
# script:
# - pip install .
# - flake8 --exit-zero --format gl-codeclimate --output-file examples-report.json --tee examples/
# - python -m json.tool < examples-report.json

deploy_local:
stage: deploy
script:
- env
- pip install twine build
- python -m build -w
- TWINE_PASSWORD=${CI_JOB_TOKEN} TWINE_USERNAME=gitlab-ci-token python -m twine upload --repository-url ${CI_API_V4_URL}/projects/${CI_PROJECT_ID}/packages/pypi dist/*

deploy_public:
stage: deploy
script:
- env
- pip install twine build
- python -m build -w
- TWINE_PASSWORD=${CI_JOB_TOKEN} TWINE_USERNAME=gitlab-ci-token python -m twine upload --repository-url ${CI_API_V4_URL}/projects/${PUBLIC_REGISTRY_PROJECT_ID}/packages/pypi dist/*
23 changes: 22 additions & 1 deletion pyramid_celery/loaders.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import datetime
import json
import pyjson5 as json5
from pyjson5 import Json5DecoderException as JSONDecodeError
import celery.loaders.base
import celery.schedules
import configparser
Expand Down Expand Up @@ -91,6 +93,25 @@ def get_route_config(parser, section):
return config


def safe_conversion(value):
"""convert a string to a more specific type"""
try:
newval= json5.loads(value)
#print("JSON DECODED", type(newval), newval)
return newval
except JSONDecodeError:
pass

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


if value.lower() in ("true", "false"):
return bool(value)
Comment on lines +97 to +98

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this, I suggest using pyramid.settings.asbool to consider the full range of supported "bool-like" values.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point about the integer. Better to stay safe with the explicit bool-like strings. 👍

try:
if float(value).is_integer():
return int(value)
return float(value)
except ValueError:
return value


class INILoader(celery.loaders.base.BaseLoader):
ConfigParser = configparser.SafeConfigParser

Expand All @@ -106,7 +127,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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


if celery_version.major > 6:
# TODO: Check for invalid settings
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
requires = ['pyramid', 'celery']

setup(name='pyramid_celery',
version='4.0.0',
version='4.0.3',
description='Celery integration with pyramid',
long_description=README + "\n" + CHANGES,
classifiers=[
Expand Down