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

scanner: Safely load default config #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jayvdb
Copy link

@jayvdb jayvdb commented Feb 20, 2020

Fixes #23

@hprid
Copy link
Member

hprid commented Feb 20, 2020

Thank you for the pull request. I am not sure whether this is a good fix; it might only defer the problem until someone has some hook that introduces uppercase module variables. Loading the configuration from a Python module was (more or less) just an easy way to get a readable configuration file with support for basic data structures and comments. Currently I consider changing the configuration file format to TOML, which fulfills the mentioned requirements without having any side effects due to code execution, hooks etc. Any opinions on that?

@jayvdb
Copy link
Author

jayvdb commented Feb 21, 2020

it might only defer the problem until someone has some hook introduces uppercase module variables

This is a vanishing/obscure small scenario.

I have 5495 packages installed in my system site-packages - almost every Python package available from openSUSE, and I have quite a large collection of packages I develop at https://build.opensuse.org/project/show/home:jayvdb:py-new , https://build.opensuse.org/project/show/home:jayvdb:django , etc

There are very few hooks in mainstream use, and people who write them either know to use dunders and peps regarding underscore prefixes for private variables, or accept it is a bug to inject variables into the exportable/public interface of other peoples modules.

I could replace .isupper() with .. != '_' and that also passes on my system.

Loading the configuration from a Python module was (more or less) just an easy way to get a readable configuration file with support for basic data structures and comments.

I fought that fight at pywikibot. Here is our singleton config module which sets defaults and loads user overrides. Very powerful, but I wouldnt go down that path again willingly.
https://github.com/wikimedia/pywikibot/blob/master/pywikibot/config2.py

https://pypi.org/project/pon/ is the closest I would go to that approach, as it at least solves most of the problems of loading Python files.

Currently I consider changing the configuration file format to TOML, ..

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: can't pickle _cffi_backend.CTypeDescr objects
2 participants