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

Run task first and detect local execution #1997

Merged
merged 4 commits into from
Nov 27, 2023

Conversation

troychiu
Copy link
Member

@troychiu troychiu commented Nov 26, 2023

Tracking issue

flyteorg/flyte#4284

Describe your changes

This PR introduces two enhancements to the Flyin VSCode decorator:

  1. Conditional VSCode Server Launching: The VSCode server launch behavior is now context-sensitive:
  • Local Execution: When a task is executed locally (using commands like python xxx.py or pyflyte run xxx), the VSCode server will not be initiated.
  • Remote Execution: In contrast, when a task is run remotely (using pyflyte run --remote ...), the VSCode server will be launched.
  1. New Argument - run_task_first: A new argument has been added to the decorator:
  • Functionality: When set to True, this argument ensures that the user's task is executed first.
  • Server Launching Condition: The VSCode server will only be launched if the user's task execution fails.

Check all the applicable boxes

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

Copy link

codecov bot commented Nov 26, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (dc50f06) 85.79% compared to head (f70d7f6) 85.82%.

Files Patch % Lines
...lyin/flytekitplugins/flyin/vscode_lib/decorator.py 70.00% 3 Missing ⚠️
plugins/flytekit-flyin/tests/test_flyin_plugin.py 96.29% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1997      +/-   ##
==========================================
+ Coverage   85.79%   85.82%   +0.02%     
==========================================
  Files         315      315              
  Lines       23351    23412      +61     
  Branches     3530     3530              
==========================================
+ Hits        20034    20093      +59     
- Misses       2713     2716       +3     
+ Partials      604      603       -1     

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

@troychiu troychiu marked this pull request as ready for review November 26, 2023 20:43
ByronHsu
ByronHsu previously approved these changes Nov 27, 2023
Signed-off-by: troychiu <[email protected]>
Signed-off-by: troychiu <[email protected]>
Signed-off-by: troychiu <[email protected]>
@eapolinario eapolinario merged commit 67b3eef into flyteorg:master Nov 27, 2023
76 checks passed
RRap0so pushed a commit to RRap0so/flytekit that referenced this pull request Dec 15, 2023
* Run task first and detect local execution

Signed-off-by: troychiu <[email protected]>

* fix log

Signed-off-by: troychiu <[email protected]>

* fix log

Signed-off-by: troychiu <[email protected]>

* lint

Signed-off-by: troychiu <[email protected]>

---------

Signed-off-by: troychiu <[email protected]>
Signed-off-by: Rafael Raposo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants