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

Overwrite SQLite3 Task image #1165

Merged
merged 4 commits into from
Sep 16, 2022
Merged

Overwrite SQLite3 Task image #1165

merged 4 commits into from
Sep 16, 2022

Conversation

pingsutw
Copy link
Member

@pingsutw pingsutw commented Sep 15, 2022

Signed-off-by: Kevin Su [email protected]

TL;DR

Users should be able to overwrite the SQL task's image.

If they don't specify the image in the SQL task, then use the default image instead.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

image

Tracking Issue

https://github.com/flyteorg/flyte/issues/

Follow-up issue

NA
OR

@codecov
Copy link

codecov bot commented Sep 15, 2022

Codecov Report

Merging #1165 (9f09f5c) into master (2ccaed7) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1165      +/-   ##
==========================================
+ Coverage   68.51%   68.53%   +0.02%     
==========================================
  Files         288      288              
  Lines       26085    26197     +112     
  Branches     2918     2924       +6     
==========================================
+ Hits        17871    17954      +83     
- Misses       7735     7765      +30     
+ Partials      479      478       -1     
Impacted Files Coverage Δ
flytekit/extras/sqlite3/task.py 90.74% <100.00%> (ø)
tests/flytekit/unit/extras/sqlite3/test_task.py 100.00% <100.00%> (ø)
tests/flytekit/unit/models/test_execution.py 100.00% <0.00%> (ø)
tests/flytekit/unit/configuration/test_internal.py 100.00% <0.00%> (ø)
flytekit/configuration/__init__.py 37.45% <0.00%> (+0.72%) ⬆️
flytekit/models/execution.py 42.20% <0.00%> (+1.14%) ⬆️
flytekit/core/shim_task.py 80.55% <0.00%> (+2.77%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Kevin Su <[email protected]>
@pingsutw pingsutw changed the title Overwrite SQLite3Task image Overwrite SQLite3 Task image Sep 15, 2022
samhita-alla
samhita-alla previously approved these changes Sep 15, 2022
Copy link
Collaborator

@eapolinario eapolinario left a comment

Choose a reason for hiding this comment

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

Looks good, just one minor comment.

@@ -88,7 +89,7 @@ def __init__(
name=name,
task_config=task_config,
# If you make changes to this task itself, you'll have to bump this image to what the release _will_ be.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you remove the comment?

Signed-off-by: Kevin Su <[email protected]>
wild-endeavor
wild-endeavor previously approved these changes Sep 16, 2022
Signed-off-by: Kevin Su <[email protected]>
@pingsutw pingsutw merged commit 495894d into master Sep 16, 2022
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.

4 participants