-
Notifications
You must be signed in to change notification settings - Fork 297
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
Add raw container (copilot) integration test #2892
base: master
Are you sure you want to change the base?
Add raw container (copilot) integration test #2892
Conversation
Signed-off-by: wayner0628 <a901639@gmail.com>
Signed-off-by: wayner0628 <a901639@gmail.com>
Signed-off-by: wayner0628 <a901639@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2892 +/- ##
===========================================
- Coverage 76.79% 43.94% -32.86%
===========================================
Files 196 199 +3
Lines 20546 20781 +235
Branches 2646 2671 +25
===========================================
- Hits 15779 9132 -6647
- Misses 4068 11416 +7348
+ Partials 699 233 -466 ☔ View full report in Codecov by Sentry. |
Signed-off-by: wayner0628 <a901639@gmail.com>
The current issue with this PR is that the integration test environment is unable to locate the Docker server needed to build the copilot image. Not sure if it is this @pytest.mark.skipif(
sys.platform in ["darwin", "win32"],
reason="Skip if running on windows or macos due to CI Docker environment setup failure",
) makes the unittest environment able to pass the test |
I tried using subprocess to build the image, but there's no docker command
|
Signed-off-by: Future-Outlier <eric901201@gmail.com>
|
||
client = docker.from_env() | ||
path_to_dockerfile = "tests/flytekit/unit/core/" | ||
dockerfile_name = "Dockerfile.raw_container" | ||
client.images.build(path=path_to_dockerfile, dockerfile=dockerfile_name, tag="flytekit:rawcontainer") | ||
|
||
flyte_file_io = ContainerTask( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the only way to do this is to build an image at CI.
We can use my image futureoutlier/rawcontainer:0320
first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @pingsutw what do you think to build your image when executing integration test?
Are these 2 the only ways?
Tracking issue
flyteorg/flyte#5930
Why are the changes needed?
To prevent in the future, someone writing code that will break the copilot.
What changes were proposed in this pull request?
Add code tests/flytekit/unit/core/test_local_raw_container.py to tests/flytekit/integration/remote/test_remote.py.
How was this patch tested?
Setup process
Screenshots
Check all the applicable boxes
Related PRs
#2887
Docs link