Preserve folder structure for app source code in bundle generate#2848
Preserve folder structure for app source code in bundle generate#2848andrewnester merged 9 commits intomainfrom
Conversation
andrewnester
requested review from
anton-107,
denik,
pietern and
shreyas-goenka
as code owners
andrewnester
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
andrewnester
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
andrewnester
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
andrewnester
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
cmd/bundle/generate/utils.go
Outdated
|
|
||
| filename := path.Base(*filePath) | ||
| targetPath := filepath.Join(n.sourceDir, filename) | ||
| if n.basePath == "" { |
There was a problem hiding this comment.
Do we need to mutate state here? Would be easier to reason about if it's just a variable assignment.
There was a problem hiding this comment.
We do need state because these functions are called recursively so we need to preserve what the basePath is
There was a problem hiding this comment.
Might have missed this, but which function is recursive here?
There was a problem hiding this comment.
Sorry, I wasn't clear there, it's markDirectoryForDownload calling markFileForDownload. We could pass basePath to markFileForDownload and markNotebookForDownload but I'd prefer to incapsulate it into state of the downloader object instead
cmd/bundle/generate/utils.go
Outdated
|
|
||
| filename := path.Base(*notebookPath) + ext | ||
| targetPath := filepath.Join(n.sourceDir, filename) | ||
| if n.basePath == "" { |
There was a problem hiding this comment.
Do we need to modify markNotebookForDownload? Seems like only markFileForDownload is called for apps generate.
There was a problem hiding this comment.
For apps - yes, but generally it makes sense to make this improvement for all types of files we donwload
There was a problem hiding this comment.
But we only call markDirectoryForDownload in apps. We also don't call markNotebookForDownload in markDirectoryForDownload.
Is there still an improvement to be had?
There was a problem hiding this comment.
Right now we don't need, this is more for consistency between the functions, so all of the respect the basePath
andrewnester
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
andrewnester
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
andrewnester
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
left a comment
shreyas-goenka
left a comment
There was a problem hiding this comment.
Thanks! Approving to unblock.
andrewnester
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
andrewnester
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
## Release v0.252.0 ### Dependency updates * Upgraded Go SDK to 0.69.0 ([#2867](#2867)) * Upgraded to TF provider 1.79.0 ([#2869](#2869)) ### Bundles * Remove unused fields from resources.models schema: creation\_timestamp, last\_updated\_timestamp, latest\_versions and user\_id. Using them now raises a warning ([#2828](#2828)). * Preserve folder structure for app source code in bundle generate ([#2848](#2848)) * Fix normalising requirements file path in dependencies section ([#2861](#2861)) * Fix default-python template not to add environments when serverless=yes and include\_python=no ([#2866](#2866)) * Fix handling of Unicode characters in Python support ([#2873](#2873)) * Add support for secret scopes in DABs ([#2744](#2744)) * Make `artifacts.*.type` optional in bundle JSON schema ([#2881](#2881)) * Fix support for `spot_bid_max_price` field in Python support ([#2883](#2883))
2 participants
Changes
Preserve folder structure for app source code in bundle generate
Why
App source code is often complex and contains many subfolders so we need to keep the folder structure so the generated app is working correctly.
Tests
Added acceptance test