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

Added SLC deployment integration tests #6

Merged
merged 25 commits into from
May 16, 2024

Conversation

ahsimb
Copy link
Collaborator

@ahsimb ahsimb commented May 14, 2024

closes #5

# Conflicts:
#	exasol/python_extension_common/deployment/language_container_deployer.py
#	exasol/python_extension_common/deployment/language_container_deployer_cli.py
#	pyproject.toml
#	test/unit/deployment/test_language_container_deployer.py
#	test/unit/deployment/test_language_container_deployer_cli.py
@ahsimb ahsimb added the feature Product feature label May 14, 2024
@ahsimb ahsimb self-assigned this May 14, 2024
.github/workflows/checks.yml Outdated Show resolved Hide resolved
test/integration/test_language_container_deployer.py Outdated Show resolved Hide resolved
test/utils/db_utils.py Show resolved Hide resolved
test/integration/test_language_container_deployer_cli.py Outdated Show resolved Hide resolved
"""
test_name: str = request.node.name
schema = test_name
language_alias = f"PYTHON3_TE_{test_name.upper()}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
language_alias = f"PYTHON3_TE_{test_name.upper()}"
language_alias = f"PYTHON3_PEC_{test_name.upper()}"

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • test_name.upper() might need sanitization should these tests every get parameterized
  • maybe extract into function, because you need to build the language_alias in many more tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, I don't think we need to do anything fancy here. We always revert the language settings. So just a constant name, like "PYTHON3_PEC", would do. Same for the schema. We only need the schema to run a test udf. We don't need to create many of them.
What concerns me is that we use the same name for the container in the bucketfs in multiple tests. If some test fails to upload the container the test, theoretically, may still pass because the container is there from the previous test. On the other hand, making an uncontrolled number of copies of the container is not a good option either. Ideally, we would delete the container at the end of each test, but I am not sure how to delete an archive from the bucketfs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You are right, we reset the language settings, so it shouldn't be an issue to use a constant.

Regarding removing a file from bucketfs, in theory the bucketfs-python has functions for that, but I am not sure, if the API we are using has it. Furthermore, is removing a file from bucketfs a bit delicate, you might need to wait a bit before you can upload the a file under the same name.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it is the best to wait for the container removal until we introduce the new bucketfs API, because it for sure supports removal. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the removal is finicky. I've been experimenting with normal files recently. If you delete a file and try to upload another file with the same name immediately after (next test), it fails. I had to wait like 10-20 seconds. I don't know if we can delete an archive. I haven't tried that. Will it delete both the tar and the unzipped directory? How long will we have to wait after that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, IT will removed both. Regarding the time or needs, that is a really good question. Probably, something we need to try and what also can be very different for SaaS.

test/integration/test_language_container_deployer.py Outdated Show resolved Hide resolved
test/integration/test_language_container_deployer.py Outdated Show resolved Hide resolved
test/integration/test_language_container_deployer_cli.py Outdated Show resolved Hide resolved
test/integration/test_language_container_deployer_cli.py Outdated Show resolved Hide resolved
test/integration/test_language_container_deployer_cli.py Outdated Show resolved Hide resolved
ahsimb and others added 11 commits May 15, 2024 16:15
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
@ahsimb ahsimb merged commit 2f561ad into main May 16, 2024
15 checks passed
@ahsimb ahsimb deleted the feature/5-slc-deployment-integration-tests branch May 16, 2024 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Product feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add integration tests for the SLC deployment
2 participants