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

Rename databricks task type #2574

Merged
merged 10 commits into from
Jul 12, 2024
Merged

Rename databricks task type #2574

merged 10 commits into from
Jul 12, 2024

Conversation

pingsutw
Copy link
Member

@pingsutw pingsutw commented Jul 9, 2024

Tracking issue

NA

Why are the changes needed?

For now, Spark tasks can only run on either k8s or Databricks, but they cannot run on both at the same time.

What changes were proposed in this pull request?

The databricks task's type (spark) should be renamed to support running both Spark and Databricks in the same workflow.

  • Task: To keep backward compatible, we add a new task config (DatabricksV2), and the new task type for this config is databricks.
from flytekitplugins.spark import Databricks # Old user

from flytekitplugins.spark import DatabricksV2 as Databricks # New user
  • Agent: Always register two agetns, these agent supports spark and databricks respectively.
AgentRegistry.register(DatabricksAgent())
AgentRegistry.register(DatabricksAgentV2())

How was this patch tested?

local/remote

Setup process

Screenshots

Screenshot 2024-07-10 at 9 47 26 PM

Check all the applicable boxes

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

Related PRs

NA

Docs link

NA

Signed-off-by: Kevin Su <[email protected]>
pingsutw added 6 commits July 9, 2024 14:09
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
@pingsutw pingsutw self-assigned this Jul 11, 2024
@pingsutw pingsutw marked this pull request as ready for review July 11, 2024 00:09
pingsutw added 2 commits July 10, 2024 17:11
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Copy link

codecov bot commented Jul 11, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 76.16%. Comparing base (ccade5a) to head (17a47b8).
Report is 1 commits behind head on master.

Files Patch % Lines
flytekit/extend/backend/base_agent.py 20.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2574      +/-   ##
==========================================
- Coverage   76.23%   76.16%   -0.07%     
==========================================
  Files         183      184       +1     
  Lines       18745    18783      +38     
  Branches     3699     3706       +7     
==========================================
+ Hits        14290    14306      +16     
- Misses       3825     3844      +19     
- Partials      630      633       +3     

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

Signed-off-by: Kevin Su <[email protected]>
@pingsutw pingsutw merged commit 993ff59 into master Jul 12, 2024
46 checks passed
fiedlerNr9 pushed a commit that referenced this pull request Jul 25, 2024
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Jan Fiedler <[email protected]>
mao3267 pushed a commit to mao3267/flytekit that referenced this pull request Jul 29, 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
Development

Successfully merging this pull request may close these issues.

3 participants