Skip to content

Async Sidecar + maintenance pytest-simcore + docker build-kit/X #1350

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

Merged

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented Mar 6, 2020

What do these changes do?

Changes to sidecar:

  • uses async libraries (aio-pika instead of pika, aiofiles, aiodocker instead of docker). This should reduce the issues where RabbitMQ closes the connections since the sync version of the sidecar was blocked.
  • in case of failure the sidecar will now update the database accordingly
  • add basic unit/integration testing infrastructure using pytest-simcore package (workflow is now tested in Travis/GithubActions)
  • uses python-3.6.10 dockerfile based on Debian (https://pythonspeed.com/articles/base-image-python-docker-images/)
  • sidecar is now following conventions for package naming (simcore-service-sidecar)
  • sidecar may be run from the command line without celery backend through cli.py
  • reduced dependencies to simcore-sdk
  • added Makefile + versioning with bump2version following Makefile conventions

Bonus simcore-sdk:

  • removed pika
  • node ports now uses aiopg/sqlalchemy.core instead of sqlalchemy.orm
  • removed some dependencies to simcore-sdk very old modules
  • uses pytest-simcore

Bonus director:

  • uses python-3.6.0 dockerfile based on Debian
  • uses pytest-simcore

Bonus pytest-simcore fixtures:

  • docker-registry small fixes
  • docker-compose was not using locally built images but production images which leads to inconsistencies in the tests
  • docker-swarm now checks for the IP (useful if more than one network interface is in use)
  • minio-service provides bucket fixture + fix S3_SECURE env
  • postgres-service follows same schema as other fixtures using a tenacitied function
  • rabbit-service: fixed unclosed connection. adds rabbit connection, channels,queues,exchanges
  • storage fixture: fix minor issue

Bonus system-testing:

  • uses pytest-simcore
  • fix syntax differences between alpine/debian based

Bonus Github Actions:

  • Reduce build time by removing unnecessary installation of docker (already available and up-to-date - check API 1.40)
  • Update docker-compose from 1.25.0 -> 1.25.3

Bonus Makefile:

  • Add support of docker buildkit make build-kit build-devel-kit build-cache-kit
  • Add support of docker buildX - used in GithubActions only for now (NOTE: probably only works on Linux)
    ./ci/github/helpers/setup_docker_buildx.bash
    ./ci/github/helpers/setup_docker_experimental.bash
    make build-x
    make build-devel-x
    make build-cache-x
    

Bonus api-gateway:

  • fixes Dockerfile issues found in sidecar

Related issue number

How to test

Checklist

  • Did you change any service's API? Then make sure to bundle document and upgrade version (make openapi-specs, git commit ... and then make version-*)
  • Unit tests for the changes exist
  • Runs in the swarm
  • Documentation reflects the changes
  • New module? Add your github username to .github/CODEOWNERS

@sanderegg sanderegg added this to the Mithos milestone Mar 6, 2020
@sanderegg sanderegg self-assigned this Mar 6, 2020
@codecov
Copy link

codecov bot commented Mar 6, 2020

Codecov Report

Merging #1350 into master will increase coverage by 0.42%.
The diff coverage is 79.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1350      +/-   ##
==========================================
+ Coverage   70.79%   71.21%   +0.42%     
==========================================
  Files         225      237      +12     
  Lines        8916     9371     +455     
  Branches      979     1027      +48     
==========================================
+ Hits         6312     6674     +362     
- Misses       2323     2397      +74     
- Partials      281      300      +19
Flag Coverage Δ
#integrationtests 59.09% <79.16%> (+1.92%) ⬆️
#unittests 65.73% <44.02%> (+0.37%) ⬆️
Impacted Files Coverage Δ
...ar/src/simcore_service_sidecar/celery_log_setup.py 0% <ø> (ø)
...kages/simcore-sdk/src/simcore_sdk/config/rabbit.py 0% <0%> (ø) ⬆️
...ices/sidecar/src/simcore_service_sidecar/celery.py 0% <0%> (ø)
...idecar/src/simcore_service_sidecar/remote_debug.py 0% <0%> (ø)
...k/src/simcore_sdk/node_ports/_schema_items_list.py 100% <100%> (ø) ⬆️
...ices/sidecar/src/simcore_service_sidecar/config.py 100% <100%> (ø)
...es/sidecar/src/simcore_service_sidecar/__init__.py 100% <100%> (ø)
...core-sdk/src/simcore_sdk/node_ports/_items_list.py 90.69% <100%> (ø) ⬆️
...simcore_service_webserver/computation_subscribe.py 97.1% <100%> (ø) ⬆️
...s/simcore-sdk/src/simcore_sdk/node_ports/config.py 100% <100%> (ø) ⬆️
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa945b2...3ee7277. Read the comment docs.

@sanderegg sanderegg force-pushed the bugfix/sidecar_check_return_value branch 2 times, most recently from 546953b to e6afe72 Compare March 15, 2020 20:31
@sanderegg sanderegg changed the title WIP: make sidecar async and set return values WIP: refactoring sidecar (async, docker, return values) Mar 16, 2020
@sanderegg sanderegg force-pushed the bugfix/sidecar_check_return_value branch from 52c1356 to 67fdbce Compare March 17, 2020 06:19
@sanderegg sanderegg force-pushed the bugfix/sidecar_check_return_value branch 2 times, most recently from c5ae4fb to 1576f82 Compare March 19, 2020 22:28
@sanderegg sanderegg changed the title WIP: refactoring sidecar (async, docker, return values) Async Sidecar + maintenance pytest-simcore + docker build-kit/X Mar 27, 2020
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

Have been continuously reviewing it during the dev.
"The sidecar is dead, long live to the sidecar" :-)
Well done!

Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

Wunderbar!

@sanderegg sanderegg merged commit 9725800 into ITISFoundation:master Mar 28, 2020
@sanderegg sanderegg deleted the bugfix/sidecar_check_return_value branch March 28, 2020 11:02
@sanderegg sanderegg linked an issue Apr 6, 2020 that may be closed by this pull request
@sanderegg sanderegg mentioned this pull request Apr 6, 2020
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.

CC services issues
3 participants