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

🐛 Fixes sudden pylint error in CI #2676

Merged
merged 3 commits into from
Dec 2, 2021

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Dec 2, 2021

What do these changes do?

Recurrent CI error with the python linter: Since a few hours, all PR show the following error in the python-linting tests:

# pylint version info
pylint 2.12.1
astroid 2.9.0
Python 3.8.12 (default, Oct 18 2021, 14:07:50) 
[GCC 9.3.0]
# Running linter
************* Module conftest
services/api-server/tests/unit/conftest.py:20:0: E0611: No name 'RWApiKeysRepository' in module '_helpers' (no-name-in-module)
services/api-server/tests/unit/conftest.py:20:0: E0611: No name 'RWUsersRepository' in module '_helpers' (no-name-in-module)

------------------------------------
Your code has been rated at 10.00/10

make: *** [Makefile:374: pylint] Error 2
Error: Process completed with exit code 2.
  • This PR fixes E0611 no-name-in-module
  • Created a script that merges all CI python-linting logs into a single file to easily compare logs from two different CIs
  • A first look at the difference between two consecutive CIs still does not show any clear evidence of why this still happens
    • pip freeze list in both CIs are identicla
    • same tooling
    • micro differences in git version

image
image

@pcrespov pcrespov changed the title Fixes sudden pylint error in CI 🐛 Fixes sudden pylint error in CI Dec 2, 2021
@pcrespov pcrespov self-assigned this Dec 2, 2021
@pcrespov pcrespov added this to the Meerkat milestone Dec 2, 2021
@codecov
Copy link

codecov bot commented Dec 2, 2021

Codecov Report

Merging #2676 (dfe66d6) into master (b929d26) will increase coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2676   +/-   ##
======================================
  Coverage    78.0%   78.0%           
======================================
  Files         636     636           
  Lines       26064   26064           
  Branches     2524    2524           
======================================
+ Hits        20341   20355   +14     
+ Misses       5053    5040   -13     
+ Partials      670     669    -1     
Flag Coverage Δ
integrationtests 66.8% <ø> (+0.1%) ⬆️
unittests 73.6% <ø> (-0.1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...rc/simcore_service_director_v2/modules/rabbitmq.py 87.7% <0.0%> (-1.8%) ⬇️
...ce_webserver/resource_manager/garbage_collector.py 73.5% <0.0%> (ø)
...ector_v2/modules/dynamic_sidecar/scheduler/task.py 86.4% <0.0%> (+0.5%) ⬆️
..._director_v2/modules/dynamic_sidecar/docker_api.py 87.2% <0.0%> (+0.5%) ⬆️
...tor_v2/modules/dynamic_sidecar/scheduler/events.py 95.0% <0.0%> (+1.4%) ⬆️
..._director_v2/modules/dynamic_sidecar/client_api.py 70.6% <0.0%> (+1.8%) ⬆️
...rector_v2/modules/comp_scheduler/base_scheduler.py 91.0% <0.0%> (+2.0%) ⬆️
...simcore_service_webserver/projects/project_lock.py 100.0% <0.0%> (+5.2%) ⬆️
...ector_v2/modules/comp_scheduler/background_task.py 86.2% <0.0%> (+7.8%) ⬆️

@mrnicegyu11
Copy link
Member

This is the changelog from the github-actions-virtual-container [from github organisation] (actions/runner-images@e8bd81b)

Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, lets get this in so the deploy-pipeline is not blocked 👯

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice tool!
One thing I do not understand is why you want to remove helpers in general. I find them quite useful to reduce file shear size.

@pcrespov
Copy link
Member Author

pcrespov commented Dec 2, 2021

very nice tool! One thing I do not understand is why you want to remove helpers in general. I find them quite useful to reduce file shear size.
@sanderegg

  • In that particular case. the helper was not reused anywhere.
  • Using helpers is a good practice but it turns bad when all helpers are copy&pastes of others and have the same name.
  • Tests do not include __init__.py which makes importing helpers with the same name unreliable

@pcrespov pcrespov merged commit 670cb22 into ITISFoundation:master Dec 2, 2021
@pcrespov pcrespov deleted the fixes/pylint-error branch December 2, 2021 17:56
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.

None yet

3 participants