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/*
20 changes: 19 additions & 1 deletion pyramid_celery/loaders.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import datetime
import json
from ast import literal_eval
import celery.loaders.base
import celery.schedules
import configparser
Expand Down Expand Up @@ -91,6 +92,23 @@ def get_route_config(parser, section):
return config


def safe_conversion(value):
"""convert a string to a more specific type"""
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:
pass

Choose a reason for hiding this comment

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

I don't think this is needed anymore since literal_eval should handle it as well.

try:
return literal_eval(value)
except ValueError:
pass
return value


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

Expand All @@ -106,7 +124,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.4',
description='Celery integration with pyramid',
long_description=README + "\n" + CHANGES,
classifiers=[
Expand Down