Page MenuHomePhabricator

eventlogging fails flake8 due to new upstream version, breaking CI
Closed, ResolvedPublic

Description

CI is failing in the eventlogging Gerrit repository:

22:37:06 flake8 installed: flake8==3.6.0,mccabe==0.6.1,pycodestyle==2.4.0,pyflakes==2.0.0
22:37:06 flake8 runtests: PYTHONHASHSEED='2471084379'
22:37:06 flake8 runtests: commands[0] | flake8 eventlogging
22:37:06   /src$ /src/.tox/flake8/bin/flake8 eventlogging 

22:37:07 eventlogging/topic.py:52:11: W605 invalid escape sequence '\/'
22:37:07 eventlogging/topic.py:52:15: W605 invalid escape sequence '\/'
22:37:07 eventlogging/handlers.py:111:5: W606 'async' and 'await' are reserved keywords starting with Python 3.7
22:37:07 eventlogging/handlers.py:197:12: W606 'async' and 'await' are reserved keywords starting with Python 3.7
22:37:07 eventlogging/handlers.py:293:20: W606 'async' and 'await' are reserved keywords starting with Python 3.7
22:37:07 eventlogging/handlers.py:313:5: W606 'async' and 'await' are reserved keywords starting with Python 3.7
22:37:07 eventlogging/handlers.py:401:12: W606 'async' and 'await' are reserved keywords starting with Python 3.7
22:37:07 eventlogging/handlers.py:479:20: W606 'async' and 'await' are reserved keywords starting with Python 3.7
22:37:07 eventlogging/utils.py:46:2: W605 invalid escape sequence '\S'
22:37:07 eventlogging/utils.py:46:5: W605 invalid escape sequence '\.'
22:37:07 eventlogging/utils.py:46:23: W605 invalid escape sequence '\.'
22:37:07 eventlogging/utils.py:46:41: W605 invalid escape sequence '\.'
22:37:07 eventlogging/utils.py:46:46: W605 invalid escape sequence '\.'
22:37:07 eventlogging/utils.py:46:46: W605 invalid escape sequence '\S'
22:37:07 eventlogging/parse.py:159:22: W605 invalid escape sequence '\w'
22:37:07 ERROR: InvocationError: '/src/.tox/flake8/bin/flake8 eventlogging'
22:37:12 flake8-bin runtests: commands[0] | flake8 --ignore=E402 --filename=* bin
22:37:12   /src$ /src/.tox/flake8-bin/bin/flake8 --ignore=E402 --filename=* bin 

22:37:12 bin/eventlogging-processor:42:80: E501 line too long (84 > 79 characters)
22:37:12 bin/eventlogging-processor:146:11: E111 indentation is not a multiple of four
22:37:12 bin/eventlogging-load-tester:87:21: W504 line break after binary operator
22:37:12 bin/eventlogging-load-tester:109:21: W504 line break after binary operator
22:37:12 bin/eventlogging-devserver:204:-379: W605 invalid escape sequence '\_'
22:37:12 bin/eventlogging-devserver:204:-353: W605 invalid escape sequence '\_'
22:37:12 bin/eventlogging-devserver:204:-253: W605 invalid escape sequence '\_'
22:37:12 bin/eventlogging-devserver:204:-223: W605 invalid escape sequence '\_'
22:37:12 bin/eventlogging-devserver:204:-217: W605 invalid escape sequence '\/'
22:37:12 bin/eventlogging-devserver:204:-196: W605 invalid escape sequence '\_'
22:37:12 bin/eventlogging-devserver:204:-191: W605 invalid escape sequence '\_'
22:37:12 bin/eventlogging-devserver:204:-186: W605 invalid escape sequence '\_'
22:37:12 bin/eventlogging-devserver:204:-181: W605 invalid escape sequence '\_'
22:37:12 bin/eventlogging-devserver:204:-165: W605 invalid escape sequence '\_'
22:37:12 bin/eventlogging-devserver:204:-43: W605 invalid escape sequence '\|'
22:37:12 bin/eventlogging-devserver:204:-38: W605 invalid escape sequence '\|'
22:37:12 bin/eventlogging-devserver:204:-22: W605 invalid escape sequence '\|'
22:37:12 ERROR: InvocationError: '/src/.tox/flake8-bin/bin/flake8 --ignore=E402 --filename=* bin'

Event Timeline

Legoktm triaged this task as High priority.Dec 20 2018, 6:39 AM
Legoktm created this task.
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 489899 had a related patch set uploaded (by Mforns; owner: Mforns):
[eventlogging@master] Adapt code to new flake8 restrictions after update

https://gerrit.wikimedia.org/r/489899

I fixed the flake8 and flake8-bin issues in the patch above.
However, Jenkins still fails, because "ImportError: No module named _mysql".
It's confusing, because tox in my local setup does not fail, and obviously the code works in prod.
But it seems Jenkins uses a different tox version or something that I can not see...
@hashar Do you know what this could be?

mforns added a project: Analytics-Kanban.
mforns moved this task from Next Up to In Progress on the Analytics-Kanban board.

The magic failure is that tox.ini is creates the virtualenv with flake8 as a dependency but without any version boundary. Hence as soon as a new flake8 is released, the job will magically fail :(

tox
[testenv:flake8]
commands = flake8 eventlogging
deps = flake8

[testenv:flake8-bin]
commands = flake8 --ignore=E402 --filename=* bin
deps = flake8

A way to avoid that is to pin flake8 to a specific version to avoid random breakages in the future. Eg:

- deps = flake8
+ deps = flake8==3.7.6

Then later on one can craft a commit that bumps the flake8 version and fix/ignore the newly introduced errors. In most case, people just use the latest version and whenever it breaks craft a fix up commit to catchup and rebase their patch on top of if :-)

eventlogging depends on mysqlclient>=1.3.7 (see https://pypi.org/project/mysqlclient/ mysqlclient on pypi). It is a wrapper that requires libmysql-client development files. On my local machine installing it fails with:

Collecting mysqlclient>=1.3.7 (from -r /home/hashar/projects/eventlogging/requirements.txt (line 6))
...
   Complete output from command python setup.py egg_info:
    /bin/sh: 1: mysql_config: not found

And I would need to install either of those two Debian packages (I run Debian Stretch):

$ apt-file search bin/mysql_config
libmariadb-dev-compat: /usr/bin/mysql_config
libmariadbclient-dev: /usr/bin/mysql_config

Regarding CI, the configuration is in integration/config.git. The workflow snippet is:

zuul/layout.yaml
- name: eventlogging
  test:
    - eventlogging-tox-docker
  gate-and-submit:
    - eventlogging-tox-docker

Which in short means: for eventlogging.git run the Jenkins job eventlogging-tox-docker. The CI job uses Docker containers to provide the test environment, that can be found in the job definition:

jjb/misc.yaml

- project:
    name: eventlogging
    jobs:
        - '{name}-tox-docker':
            docker_image_var: docker-registry.wikimedia.org/releng/tox-eventlogging:0.3.2

The Docker container can be pulled (docker pull docker-registry.wikimedia.org/releng/tox-eventlogging:0.3.2). It is defined in files under ./dockerfiles. Roughly:

tox-eventlogging
FROM {{ "tox" | image_tag }}

USER root
RUN {{ "librdkafka-dev" | apt_install }}

USER nobody
tox
FROM {{ "ci-stretch" | image_tag }}
{% set packages|replace('\n', ' ') -%}
python-pip
python-dev
python-wheel
python3-pip
python3-dev
python3.4-dev
python3.6-dev
python3.7-dev
python3-wheel
pypy
gcc
g++
libc-dev
make
default-libmysqlclient-dev
libssl-dev
{%- endset -%}

RUN {{ packages | apt_install }} \

default-libmysqlclient-dev should thus be available. Inspecting the container it is indeed present :) So I am not sure what is wrong

ImportError: No module named _mysql is due to mysqlclient python module, eventlogging is not compatible with 1.4.0 or later. Thus the version has to be bound to <1.4.0.

I think in Debian that is the package python-mysqldb and Stretch ships 1.3.7. Maybe we can just pin to that version?

TLDR:

--- a/requirements.txt
+++ b/requirements.txt
@@ -3,7 +3,7 @@ jsonschema>=0.7
 confluent-kafka>=0.9.1.2
 kafka-python>=1.3.0
 mock==1.0.0
-mysqlclient>=1.3.7
+mysqlclient>=1.3.7,<1.4.0
 pygments>=1.5
 psutil>=2.1.1
 PyYAML>=3.10

And it is worth adding a task to make eventlogging compatible with both 1.3 and 1.4. Bonus for switching eventlogging to use the pure python module PyMySQL!

Change 491783 had a related patch set uploaded (by Hashar; owner: Hashar):
[eventlogging@master] Use version upper bound for mysqlclient/flake8

https://gerrit.wikimedia.org/r/491783

Change 491784 had a related patch set uploaded (by Hashar; owner: Hashar):
[eventlogging@master] Add python3.5 to test, skip missing python versions

https://gerrit.wikimedia.org/r/491784

Change 491785 had a related patch set uploaded (by Hashar; owner: Hashar):
[eventlogging@master] Fix flake8 issues in bin/eventlogging-processor

https://gerrit.wikimedia.org/r/491785

@mforns I have send a bunch of patches. Some might be conflicting with the one you have send to setup flake8 :-] Feel free to take them over.

@hashar Thanks a lot for all the dedicated work!!

Change 492998 had a related patch set uploaded (by Hashar; owner: Hashar):
[eventlogging@refs/meta/config] Allow JenkinsBot to submit changes

https://gerrit.wikimedia.org/r/492998

Change 492998 merged by Hashar:
[eventlogging@refs/meta/config] Allow JenkinsBot to submit changes

https://gerrit.wikimedia.org/r/492998

Change 491783 merged by Hashar:
[eventlogging@master] Use version upper bound for mysqlclient/flake8

https://gerrit.wikimedia.org/r/491783

Change 491784 merged by jenkins-bot:
[eventlogging@master] Add python3.5 to test, skip missing python versions

https://gerrit.wikimedia.org/r/491784

I went ahead and just self merged all the patches that fix up flake8. They were all trivial / test only as far as I could tell :)

mforns awarded a token.

Change 489899 abandoned by Mforns:
Adapt code to new flake8 restrictions after update

Reason:
This is taken care in other patches, see task

https://gerrit.wikimedia.org/r/489899

Change 491785 abandoned by Hashar:
Fix flake8 issues in bin/eventlogging-processor

https://gerrit.wikimedia.org/r/491785