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 launch condition column #2204

Merged
merged 11 commits into from
Feb 29, 2024
Merged

Add launch condition column #2204

merged 11 commits into from
Feb 29, 2024

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented Feb 21, 2024

Why are the changes needed?

This relies on the changes to IDL in: https://github.com/flyteorg/flyte/pull/4903/files

What changes were proposed in this pull request?

  • Remove bindings when embeding as query, only one binding is allowed now.
  • Introduce a launch plan trigger base protocol, and add the schedule implementation of it (backend code is not ready yet so it'll throw an error for now).
  • Replace poorly named additional_metadata with trigger.
  • Removing some underscores in schedule model.
  • Update translator to call to flyte idl on the implementation of the launch plan trigger base, with the python launch plan object itself as an arg.

How was this patch tested?

End-to-end testing with private backend will be run.

Setup process

Screenshots

Check all the applicable boxes

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

Related PRs

Docs link

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Copy link

codecov bot commented Feb 21, 2024

Codecov Report

Attention: Patch coverage is 81.57895% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 85.85%. Comparing base (95ea92d) to head (a8d8424).
Report is 3 commits behind head on master.

Files Patch % Lines
flytekit/tools/translator.py 33.33% 3 Missing and 1 partial ⚠️
flytekit/core/schedule.py 72.72% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2204      +/-   ##
==========================================
+ Coverage   85.83%   85.85%   +0.02%     
==========================================
  Files         314      314              
  Lines       24027    24042      +15     
  Branches     3641     3643       +2     
==========================================
+ Hits        20623    20641      +18     
+ Misses       2762     2760       -2     
+ Partials      642      641       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

squiishyy
squiishyy previously approved these changes Feb 26, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by maintainer label Feb 26, 2024
Signed-off-by: Yee Hing Tong <[email protected]>
@wild-endeavor wild-endeavor marked this pull request as ready for review February 27, 2024 02:51
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Feb 27, 2024
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Feb 27, 2024
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
@wild-endeavor wild-endeavor merged commit 90324e0 into master Feb 29, 2024
89 of 90 checks passed
fiedlerNr9 pushed a commit that referenced this pull request Jul 25, 2024
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Jan Fiedler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants