arachnys / cabot

Self-hosted, easily-deployable monitoring and alerts service - like a lightweight PagerDuty
MIT License
5.59k stars 594 forks source link

add content conversion to unicode if needed #556

Closed maks-us closed 7 years ago

maks-us commented 7 years ago

1) The issue: I use Cyrillic website. The match check doesn't work for it. I debug the issue and I found the problem source. Django by default represent model fields as UTF-8 unicode . The library "content" is represented as string. (python 2 default docker container).

2) Solution: It will convert content string to UTF-8 unicode if needed.

JeanFred commented 7 years ago

That looks reasonable. Thanks @maks-us for reporting this, and proposing a patch!

Would you be able to add a test for this?

maks-us commented 7 years ago

@JeanFred
How can i run tests on local machine? I install tox and try to run it in project root directory. Is it expected output?

maks:cabot/ (master*) $ tox                                                                                                [22:45:38]
config installed:
config runtests: PYTHONHASHSEED='948047437'
config runtests: commands[0] | /bin/bash -c set -euo pipefail && . ./conf/production.env.example
config runtests: commands[1] | /bin/bash -c set -euo pipefail && . ./conf/development.env.example
flake8 installed: configparser==3.5.0,enum34==1.1.6,flake8==3.4.1,flake8-debugger==1.4.0,mccabe==0.6.1,pycodestyle==2.3.1,pyflakes==1.5.0
flake8 runtests: PYTHONHASHSEED='948047437'
flake8 runtests: commands[0] | flake8
______________________________________________________________ summary _______________________________________________________________
  config: commands succeeded
  flake8: commands succeeded
  congratulations :)
JeanFred commented 7 years ago

@maks-us Right, that’s a bit confusing, but tox only runs the linting. For the testing part, you can run the tests in Docker − see the documentation.

Alternatively, you can always push to your PR and Travis will run them for you ;-)

maks-us commented 7 years ago

@JeanFred done

JeanFred commented 7 years ago

Great job @maks-us ! I checked locally and can confirm the tests fail without your fix, and pass with it. I’ll squash and merge this.