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

♻️ Maintenance: Removed deprecation warnings and marks flaky tests #3085

Merged
merged 13 commits into from
Jun 10, 2022

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jun 2, 2022

What do these changes do?

Misc maintenance: Various minor issues in the repo.

Mostly warnings displayed during test or execution

  • 🐛 adds flaky to servicelib and mark test_context_aware_measure_parallelis @GitHK please check
  • 🔨 fixes DeprecationWarningThe 'asyncio_mode' is 'legacy', switching to 'auto' for the sake of pytest-aiohttp backward compatibility. Please explicitly use 'asyncio_mode=strict' or 'asyncio_mode=auto' in pytest configuration file by adding configuration insetup.cfg
  • 🔨 fixes PytestWarning
  • 🔨 fixes UserWarning _client.py:2012: nclosed <httpx.AsyncClient
  • 🗑️ removes models_library.settings (now should use settings_library instead)
  • ⬆️ upgrades paramiko to avoid Warnings due to Blowfish deprecation in cryptography
# name before after upgrade count packages
1 paramiko 2.10.4 2.11.0 minor 4 director-v2🧪
postgres-database🧪
service-library🧪
web🧪

Legend:

  • ⬆️ base dependency (only services because packages are floating)
  • 🧪 test dependency
  • 🔧 tool dependency

@pcrespov pcrespov self-assigned this Jun 2, 2022
@codecov
Copy link

codecov bot commented Jun 2, 2022

Codecov Report

Merging #3085 (7ff1308) into master (9bbe28c) will increase coverage by 1.8%.
The diff coverage is 65.0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3085     +/-   ##
========================================
+ Coverage    79.2%   81.0%   +1.8%     
========================================
  Files         716     715      -1     
  Lines       30924   30922      -2     
  Branches     4035    4035             
========================================
+ Hits        24494   25052    +558     
+ Misses       5524    4989    -535     
+ Partials      906     881     -25     
Flag Coverage Δ
integrationtests 66.2% <100.0%> (+0.8%) ⬆️
unittests 76.9% <65.0%> (+0.1%) ⬆️

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

Impacted Files Coverage Δ
...re_sdk/node_ports_common/client_session_manager.py 100.0% <ø> (ø)
...g/src/simcore_service_catalog/services/director.py 82.9% <61.1%> (-2.6%) ⬇️
...simcore-sdk/src/simcore_sdk/config/http_clients.py 100.0% <100.0%> (ø)
...catalog/src/simcore_service_catalog/core/events.py 90.0% <100.0%> (ø)
.../simcore_service_catalog/db/repositories/groups.py 70.2% <0.0%> (-5.5%) ⬇️
...mcore_service_webserver/garbage_collector_utils.py 79.4% <0.0%> (-3.0%) ⬇️
.../simcore_service_catalog/services/access_rights.py 78.7% <0.0%> (-2.5%) ⬇️
.../director/src/simcore_service_director/producer.py 61.6% <0.0%> (-0.3%) ⬇️
.../src/simcore_service_webserver/director_v2_core.py 76.7% <0.0%> (+0.9%) ⬆️
...webserver/computation_comp_tasks_listening_task.py 77.6% <0.0%> (+1.0%) ⬆️
... and 45 more

../../.venv/lib/python3.9/site-packages/pytest_aiohttp/plugin.py:28
  /home/crespo/devp/osparc-simcore/.venv/lib/python3.9/site-packages/pytest_aiohttp/plugin.py:28: DeprecationWarning: The 'asyncio_mode' is 'legacy', switching to 'auto' for the sake of pytest-aiohttp backward compatibility. Please explicitly use 'asyncio_mode=strict' or 'asyncio_mode=auto' in pytest configuration file.
    config.issue_config_time_warning(LEGACY_MODE, stacklevel=2)
…ated, use instead settings_library.http_client_request
…marked with '@pytest.mark.asyncio' but it is not an async function. Please remove asyncio marker. If the test is not marked explicitly, check for global markers applied via 'pytestmark'.
@pcrespov pcrespov changed the title WIP ♻️ Misc Maintenance/week 22 ♻️ Removed deprecation warnings and marks flaky tests Jun 8, 2022
@pcrespov pcrespov changed the title ♻️ Removed deprecation warnings and marks flaky tests ♻️ Maintenance: Removed deprecation warnings and marks flaky tests Jun 8, 2022
@pcrespov pcrespov added the t:maintenance Some planned maintenance work label Jun 8, 2022
@pcrespov pcrespov added this to the Croissant+1 milestone Jun 8, 2022
@pcrespov pcrespov marked this pull request as ready for review June 8, 2022 19:23
@pcrespov pcrespov requested a review from mguidon June 8, 2022 19:23
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.

thanks for taking care of this!



async def close_director(app: FastAPI) -> None:
if director_client := app.state.director_api:
await director_client.delete()
client: Optional[DirectorApi]
Copy link
Member

Choose a reason for hiding this comment

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

what is the point of declaring this variable?

Copy link
Member Author

Choose a reason for hiding this comment

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

this way we can get autocompletion, search, etc in vscode

@@ -6,3 +6,6 @@ tag = False
commit_args = --no-verify

[bumpversion:file:VERSION]

[tool:pytest]
asyncio_mode = auto
Copy link
Member

Choose a reason for hiding this comment

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

why was this necessary? it is already in the makefile right?

Copy link
Member Author

@pcrespov pcrespov Jun 9, 2022

Choose a reason for hiding this comment

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

is it? I need to double check then. There was a warning in the tests ...

../../.venv/lib/python3.9/site-packages/pytest_aiohttp/plugin.py:28
  /home/crespo/devp/osparc-simcore/.venv/lib/python3.9/site-packages/pytest_aiohttp/plugin.py:28: DeprecationWarning: The 'asyncio_mode' is 'legacy', switching to 'auto' for the sake of pytest-aiohttp backward compatibility. Please explicitly use 'asyncio_mode=strict' or 'asyncio_mode=auto' in pytest configuration file.
    config.issue_config_time_warning(LEGACY_MODE, stacklevel=2)

Copy link
Member Author

Choose a reason for hiding this comment

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

But yes, I see that the scripts in ci and the makefile recipes incorporate this option in CLI
That is strange then. Perhaps I was running pytest manually via CLI.

@pcrespov pcrespov requested a review from GitHK June 9, 2022 07:15
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

Please consider my proposal

packages/service-library/tests/test_async_utils.py Outdated Show resolved Hide resolved
* fixed test

* refactor test

Co-authored-by: Andrei Neagu <[email protected]>
@pcrespov pcrespov requested a review from GitHK June 10, 2022 12:59
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

👍

@sonarcloud
Copy link

sonarcloud bot commented Jun 10, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.5% 0.5% Duplication

@pcrespov pcrespov merged commit e96b27b into ITISFoundation:master Jun 10, 2022
@pcrespov pcrespov deleted the maintenance/week-22 branch June 10, 2022 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:maintenance Some planned maintenance work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants