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

fix: generate the bundle path including prefix/suffix if available #20

Merged

Conversation

pariksheet
Copy link
Collaborator

currently BRICKFLOW_WORKFLOW_SUFFIX does not work as expected.

if user deploys the workflow using suffix e.g. _suffix10
BRICKFLOW_WORKFLOW_SUFFIX=_suffix10 poetry run brickflow deploy --deploy-mode bundle --workflows-dir src/workflows -e local

then bundle root path would be - /Users/${workspace.current_user.userName}/.brickflow_bundles/test-project/local

if user deploys the same workflow using another suffix e.g. _suffix20
BRICKFLOW_WORKFLOW_SUFFIX=_suffix poetry run brickflow deploy --deploy-mode bundle --workflows-dir src/workflows -e local

then it will replace the earlier workflow because bundle root path will be generated again to be - /Users/${workspace.current_user.userName}/.brickflow_bundles/test-project/local

This fix will create dedicated bundle root path for each suffixes.
e.g _suffix10 -> /Users/${workspace.current_user.userName}/.brickflow_bundles/test-project/local/_suffix10
e.g _suffix20 -> /Users/${workspace.current_user.userName}/.brickflow_bundles/test-project/local/_suffix20

This way two different workflows will be deployed on databricks with their own lifecycle states.

Note: same concept applied for BRICKFLOW_WORKFLOW_PREFIX as well.

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02% 🎉

Comparison is base (92390c7) 88.42% compared to head (3c5b5ab) 88.45%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #20      +/-   ##
==========================================
+ Coverage   88.42%   88.45%   +0.02%     
==========================================
  Files          22       22              
  Lines        3163     3169       +6     
==========================================
+ Hits         2797     2803       +6     
  Misses        366      366              
Files Changed Coverage Δ
brickflow/codegen/databricks_bundle.py 90.03% <100.00%> (+0.20%) ⬆️

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

pyproject.toml Outdated Show resolved Hide resolved
@asingamaneni asingamaneni self-requested a review August 25, 2023 21:27
Copy link
Collaborator

@asingamaneni asingamaneni left a comment

Choose a reason for hiding this comment

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

LGTM

@asingamaneni asingamaneni merged commit e925c58 into Nike-Inc:main Aug 25, 2023
4 checks passed
@pariksheet pariksheet deleted the fix/brickflow_suffix_workspace_state branch July 2, 2024 19:18
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.

2 participants