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

Add GA workflow to test Rust Remote without installing Python gRPC #2415

Merged

Conversation

austin362667
Copy link
Contributor

@austin362667 austin362667 commented May 14, 2024

Tracking issue

flyteorg/flyte#5344

Why are the changes needed?

To add a integration test for the Rust Remote Client, we have to add Github Action Jobs for building and installing compiled Python extension, flyrs, from Rust through PyO3 bindings.

What changes were proposed in this pull request?

Caution

This PR depends on the previous work done in #2414 .
It cannot be executed before #2414 is merged.

We add two CI/CD jobs,

  1. build-without-grpcio
    • Run Rust Remote without installing Python grpcio, grpcio-status.
  2. build-with-grpcio
    • Make sure the outputs of the Rust Remote and Python remote are the same.

Both of them are only being tested against integration test test_rust_remote.py that installed flyrs via maturin develop (compiled and installed), and we skip tests if not found module flyrs.

How was this patch tested?

  1. Run integration test by command:
    pytest tests/flytekit/integration/remote/test_rust_remote.py

  2. Github Action has been tested locally (on my personal forked repo's GA CI), please check the following screenshots.

    These two newly added CI jobs should run after rebase onto the previous works flyrs.

Setup process

Caution

This PR depends on the previous work done in #2414 It cannot be merged before #2414 is merged.

Screenshots

  1. Run without gRPC, skip tests in test_rust_remote.py if no Python grpc dependencies installed.
Screenshot 2024-05-10 at 7 59 25 PM Screenshot 2024-05-10 at 7 58 35 PM
  1. Run with gRPC to compare the high-level behavior between two different implementations of the Remote Client.
Screenshot 2024-05-10 at 7 58 52 PM

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

This PR depends on the previous work #2414.

Docs link

@austin362667 austin362667 force-pushed the austin362667/flyrs/remote/ci_jobs branch 7 times, most recently from 2043481 to 78b1577 Compare May 15, 2024 10:07
@austin362667 austin362667 force-pushed the austin362667/flyrs/remote/ci_jobs branch from bdf9501 to 68fc16a Compare May 19, 2024 08:18
@austin362667 austin362667 marked this pull request as draft May 21, 2024 07:23
@austin362667 austin362667 force-pushed the austin362667/flyrs/remote/ci_jobs branch 10 times, most recently from bd3e74b to b1e64e1 Compare May 21, 2024 18:46
@austin362667 austin362667 marked this pull request as ready for review May 21, 2024 19:10
@austin362667 austin362667 force-pushed the austin362667/flyrs/remote/ci_jobs branch from bef8f95 to 62cde50 Compare May 22, 2024 13:49
@pingsutw pingsutw merged commit 66c0f02 into flyteorg:flyrs May 28, 2024
48 of 49 checks passed
austin362667 added a commit to austin362667/flytekit that referenced this pull request Jun 15, 2024
austin362667 added a commit to austin362667/flytekit that referenced this pull request Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants