Fixes #135 - Add a job to fetch Webcompat Knowldege Base related bugs and store them in BQ#136
Conversation
scholtzan
left a comment
There was a problem hiding this comment.
looks good to me. someone should verify that the bugzilla import logic is doing what it's supposed to though.
CI is currently not running since the PR is coming from a fork. Once someone has reviewed the logic I can trigger the CI manually to get it to pass.
After this is merged and a docker image gets pushed, you can schedule running the docker container in telemetry-airflow. There are already some examples which you could more or less just copy-paste, like this one: https://github.com/mozilla/telemetry-airflow/blob/main/dags/search_term_data_validation.py
|
Thanks, Anna! I got some feedback during our sync meeting regarding bugzilla logic and will work on updating the code shortly. |
|
The changes we discussed are as follows:
|
denschub
left a comment
There was a problem hiding this comment.
This looks good! And the data that's already there also looks perfect. We do probably need to iterate a bit over time, but this is a really really good start.
| RUN groupadd --gid ${USER_ID} ${GROUP_ID} && \ | ||
| useradd --create-home --uid ${USER_ID} --gid ${GROUP_ID} --home-dir ${HOME} ${GROUP_ID} | ||
|
|
||
| WORKDIR ${HOME} |
There was a problem hiding this comment.
This is only a minor nit, but it's generally a good idea to reduce the number of layers for container images. While it doesn't matter much here as the images don't get pushed to a registry, it's probably not a bad idea to take care of that.
You can reduce the images quite a bit by changing the user first, then copy'ing all the files while doing a chown at the same time, and then combining all the RUNs together:
WORKDIR ${HOME}
USER ${USER_ID}
COPY --chown=${USER_ID}:${GROUP_ID} . .
RUN pip install --upgrade pip && \
pip install -r requirements.txt && \
pip install .There was a problem hiding this comment.
I haven't modified this file since it was generated by create_job script based on a template that every job in this repository follows, so it's probably fine to leave it as is.
…d bugs from bugzilla and store them in BQ
|
Thanks for the review @denschub!
@scholtzan would you be able to trigger CI, please? |
|
Looks like flake isn't quite happy, yet. |
|
Oh thanks. I've just pushed a change that sets a version of flake8 in requirements.txt (same issue as in #68), so it should pass now. |
Checklist for reviewer:
referenced, the pull request should include the bug number in the title)
.circleci/config.yml) will cause environment variables (particularlycredentials) to be exposed in test logs
telemetry-airflow
responsibly.