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

vdk-plugins: fix build of multiple plugins #2445

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

antoniivanov
Copy link
Collaborator

@antoniivanov antoniivanov commented Jul 24, 2023

There were overall 3 issues that needed to be fixed in the build of those plugins:

The first one is 2.0 major release of pytest-docker (https://pypi.org/project/pytest-docker/#history) which we use for starting docker container in some plugin tests . This impacted us because

  1. pytest-docker change default command from docker-compose to docker compose (no dash)
  2. But the library we use (docker-compose) installs only docker-compose command (with a dash)

--
The other problematic library is the installation of docker-compose itself :).
docker-compose depends on PyYaml and PyYAML is having the following issue: yaml/pyyaml#724 (in short a recent release of Cython 3.0 broke PyYAML)

As a workaround, we can pin PyYAML==5.3.1 And also remove eager (transitive) upgrades when installing the package with pip to reduce the chance of accidentally upgrading it.

--
Finally removed all non- testing dependencies from requirements.txt. In one of the plugin was failing to install because it was missing install requirements in setup.py and was not caught during testing because requirements.txt was installing that dependency.


An alternative approach to fix it might have been to switch to use testcontainer (like vdk-trino uses) instead of pytest-docker + docker-compose. But that would require refactoring of the tests so I opted out for now.

@antoniivanov antoniivanov force-pushed the person/aivanov/vdk-plugins branch 8 times, most recently from f1b8241 to 9903824 Compare July 25, 2023 06:22
@antoniivanov antoniivanov marked this pull request as ready for review July 25, 2023 06:43
@antoniivanov antoniivanov force-pushed the person/aivanov/vdk-plugins branch from 9903824 to 0b5c7da Compare July 25, 2023 06:46
@antoniivanov antoniivanov changed the title vdk-plugins: fix build vdk-plugins: fix build of multiple plugins Jul 25, 2023
@antoniivanov antoniivanov merged commit 116b6c7 into main Jul 25, 2023
@antoniivanov antoniivanov deleted the person/aivanov/vdk-plugins branch July 25, 2023 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants