Add source attribute to dbt_task and sql_task.file tasks to support files from workspace
#3208
Conversation
…port files from workspace Also added support for it in TF exporter
alexott
requested review from
mgyucht and
gaborratky-db
and removed request for
a team
There was a problem hiding this comment.
Added suggestions for docs and a few questions/requests to clarify what and why the code does what it does.
Is there test coverage for Import the PR could extend? Logic like len(parts) >= 4 feel brittle and the assumptions could break about how a path looks like in the future.
Co-authored-by: Gabor Ratky <gabor.ratky@databricks.com>
Co-authored-by: Gabor Ratky <gabor.ratky@databricks.com>
Co-authored-by: Gabor Ratky <gabor.ratky@databricks.com>
Co-authored-by: Gabor Ratky <gabor.ratky@databricks.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3208 +/- ##
==========================================
+ Coverage 83.57% 83.75% +0.17%
==========================================
Files 168 170 +2
Lines 15021 15334 +313
==========================================
+ Hits 12554 12843 +289
- Misses 1729 1738 +9
- Partials 738 753 +15
|
| @@ -240,6 +247,7 @@ func (ic *importContext) emitNotebookOrRepo(path string) { | |||
| if strings.HasPrefix(path, "/Repos") { | |||
| ic.emitRepoByPath(path) | |||
| } else { | |||
| // TODO: strip /Workspace prefix if it's provided | |||
There was a problem hiding this comment.
Question: Is this TODO not required currently? what happens if /Workspace prefix is provided?
There was a problem hiding this comment.
it's related to this issue: #3195 - we're introduced ability to refer to objects with /Workspace and without, but notebooks/workspace files right now are using path without /Workspace, so I just left a comment for myself to handle this case in the near future
| Value: strings.Join(parts[:4], "/"), | ||
| }) | ||
| } else { | ||
| log.Printf("[WARN] Incorrect Repos path") |
There was a problem hiding this comment.
instead of warn, shouldn't we Fail here?
There was a problem hiding this comment.
Unfortunately, our APIs aren't strictly consistent, so it's possible to create jobs pointing to non-existing paths, clusters, etc., but we should tolerate that :-( Otherwise, a lot of exports will fail - it's easier to fix TF code sometimes than fix objects in the workspace
Changes
Also added support for it in TF exporter
Tests
make testrun locallydocs/folderinternal/acceptance