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

locale from strategyfile will always be overwritten #78

Closed
derfryday opened this issue Jun 21, 2021 · 4 comments
Closed

locale from strategyfile will always be overwritten #78

derfryday opened this issue Jun 21, 2021 · 4 comments

Comments

@derfryday
Copy link
Contributor

derfryday commented Jun 21, 2021

Describe the bug
You can't set the locale via the strategyfile because it will always be overwritten by the fallback locale

To Reproduce
Steps to reproduce the behavior:

  1. set the locale in a strategyfile to some locale with some exclusive faker modules i.e. de_DE
  2. use a faker module exclusive to that locale i.e. state
  3. run pynonymizer without the --fake-locale argument but enable verbose to see that the locale will fall back to en_GB

Expected behavior
settings the locale in the strategyfile results in that locale being used by pynonmizer.

Additional context
The issue seemingly lies in pynonymizer/pynonymize.py:47

    if fake_locale is None:
        fake_locale = "en_GB"

fake_locale will never be None beyond this point, it's either a value specified by the CLI argument/environment variable or the fallback en_GB

the parse_config method relies on local_override to be None to use the value set in the strategyfile.

pynonymizer/strategy/parser.py:162

    def parse_config(self, raw_config, locale_override=None):
        """
        parse a configuration dict into a DatabaseStrategy.
        :param raw_config:
        :return:
        """
        config = StrategyParser.__normalize_table_list(deepcopy(raw_config))

        locale = config.get("locale", None)
        if locale_override:
            locale = locale_override

I'd suggest to put the fallback to en_GB into the line where the locale is being read from the strategyfile instead of the fallback logic from pynonymizer/pynonymize.py:47.

    def parse_config(self, raw_config, locale_override=None):
        """
        parse a configuration dict into a DatabaseStrategy.
        :param raw_config:
        :return:
        """
        config = StrategyParser.__normalize_table_list(deepcopy(raw_config))

---        locale = config.get("locale", None)
+++        locale = config.get("locale", "en_GB")
        if locale_override:
            locale = locale_override

I've created a pull request with the suggested changes: #79

@rwnx
Copy link
Owner

rwnx commented Jun 21, 2021

Thank you for this report (and fix!) this is a case of building for one set of defaults and then setting a different bunch of values in the CLI, and not really accounting for either 😅 oops.

There's definitely other places this can happen that are due an update but locale is the first one to actually get overriden, so it makes sense! I'll pick up on your PR.

@rwnx
Copy link
Owner

rwnx commented Jun 21, 2021

seeing as this patches some broken behaviour i'm going to package as patchv1.21.1, and ship over the next day or so! I'll update here.

@derfryday
Copy link
Contributor Author

lovely, thanks!

@rwnx
Copy link
Owner

rwnx commented Jun 22, 2021

I've released v1.21.1 which should contain this fix. If you could take a look and report/close this issue if it's resolved, that would be fantastic 😇

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

No branches or pull requests

2 participants