Hey everyone! :wave: I've been working on adding D...
# contribute
a
Hey everyone! 👋 I've been working on adding Databricks Serverless Compute and Unity Catalog support to the Databricks connector. We collaborated with the Databricks team on this, and it's now tested and deployed in our environment. What it enables: • Faster job startup (15 min → under 1 min with serverless) • Fine-grained, multi-tenant authentication via Unity Catalog Issues: • <http://• https://github.com/flyteorg/flyte/issues/6911|[Core feature] Add Databricks Serverless Compute Support to Databricks Connector> • <http://• https://github.com/flyteorg/flyte/issues/6912|[Core feature] Fine-Grained Token Support for Multi-Tenant Databricks Authentication> PRs:Add Databricks Serverless Compute SupportAdd serverless-compatible entrypoint for DatabricksAdd multi-tenant Databricks token support via cross-namespace K8s secretsAdd RBAC support for cross-namespace secret reading Happy to provide any additional context for review. Feedback welcome. cc: @freezing-airport-6809 /@victorious-zoo-70738 /@mysterious-country-97979 /@helpful-mechanic-41804
👍 4
f
cc @echoing-account-76888 / @glamorous-carpet-83516 can you help with the PRs?
👍 1
e
Sure! I'll help on this 🙌
a
Thanks, @echoing-account-76888 let me know if you want any other details from my end.
❤️ 1
Thanks @echoing-account-76888 and @tall-lock-23197 for reviewing the PR, I have addressed the review comments, let me know if changes looks good now.
g
@adventurous-ability-21671 thanks for opening PRs, could you also fix the lint errors in this PR https://github.com/flyteorg/flytekit/pull/3394
and you need to run “make helm” to generate some value.yaml files for this PR https://github.com/flyteorg/flyte/pull/6919
a
Thanks @glamorous-carpet-83516 for reviewing the PRs I have done following: • 3394 - Fixed the lint issue and pushed the changes. • 6919 - Ran the
make helm
which generated HELM Docs and updated the
values.yaml
Let me now, if changes looks good now.
g
https://github.com/flyteorg/flytekit/pull/3394 still has lint errors. you could run
make lint
to test it locally
e
@adventurous-ability-21671 Thanks for the PR! https://github.com/flyteorg/flytekit/pull/3392 overall LGTM, just left 2 small comments. Could you also fix the lint errors in this PR?
https://github.com/flyteorg/flyte/pull/6919 still needs
make helm
. This is a bit tricky, may need to run
make helm
-> commit -> run
make helm
again and ensure there's no errors, otherwise needs to commit again
I also left few comments under https://github.com/flyteorg/flytetools/pull/94
a
Hey @glamorous-carpet-83516, Thanks for re running the pipeline, updated PR #3394 with the lint fix. The issue was a missing
pydoclint-errors-baseline.txt
this file tracks pre-existing docstring violations repo-wide (per #3077) and was empty on our branch. Restored it from `master`; pydoclint auto-shrunk it by 4 lines since our code actually fixed the DatabricksV2 docstring. No new violations introduced.
make lint
passes cleanly now.
Copy code
$ make lint                 
mypy flytekit/core
Success: no issues found in 52 source files
mypy flytekit/types
Success: no issues found in 23 source files
mypy --allow-empty-bodies --disable-error-code="annotation-unchecked" tests/flytekit/unit/core
Success: no issues found in 102 source files
pre-commit run --all-files
ruff.....................................................................Passed
ruff-format..............................................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
shellcheck...............................................................Passed
Check for exposed PDB statements.........................................Passed
codespell................................................................Passed
pydoclint................................................................Passed
Hey @echoing-account-76888 Thanks for reviewing the PRs again, I have addressed the comments on PRs as well as fix the lint issue, let me know if all looks good now.
Hello @glamorous-carpet-83516 /@echoing-account-76888, Hope your week is off to a good start. Would like to followup on my above comments if you guys get a chance to review them again.
f
Hey man sorry but I will ask them it lgtm they should merge. Cc @echoing-account-76888 / @glamorous-carpet-83516 please help him merge
👍 1
e
Hi @adventurous-ability-21671, Sorry for the delay! I approved and merged all your PRs except https://github.com/flyteorg/flytekit/pull/3394, as there's conflict after merging your #3392. Could you please solve the conflict? Thank you!
a
Hey @echoing-account-76888 Thanks for merging the PRs, I have resolved the conflicts as well as I can see now CI is passing, can you please check 3349 again?
e
@adventurous-ability-21671 merged! Thank you!
a
Thanks @echoing-account-76888 for merging it 🙏
🙌 1