Skip to content

Add support for cloning repositories - #544

Merged
shreyas-goenka merged 22 commits into
mainfrom
git-clone-clone
Jul 25, 2023
Merged

Add support for cloning repositories#544
shreyas-goenka merged 22 commits into
mainfrom
git-clone-clone

Conversation

@shreyas-goenka

Copy link
Copy Markdown
Contributor

Changes

Adds support for cloning public and private github repositories for databricks templates

Tests

Integration tests

@shreyas-goenka

Copy link
Copy Markdown
ContributorAuthor

This PR is a replacement for #534

@shreyas-goenkashreyas-goenka changed the title Add support for cloning repositories[DRAFT] Add support for cloning repositoriesJul 4, 2023
@shreyas-goenka

Copy link
Copy Markdown
ContributorAuthor

Please leave any feedback you have, if this looks good, then I'll proceed to add some unit tests for the arg parsing and command construction

Comment threadinternal/git_clone_test.go Outdated
Comment threadlibs/git/clone.go Outdated
Comment threadlibs/cmdio/io.go Outdated
Comment threadlibs/git/clone.go Outdated
Comment threadlibs/git/clone.go Outdated
Comment threadlibs/git/clone.go Outdated
Comment threadinternal/git_clone_test.go
@shreyas-goenkashreyas-goenka changed the title [DRAFT] Add support for cloning repositoriesAdd support for cloning repositoriesJul 12, 2023
Comment threadlibs/git/clone.go Outdated
Comment threadlibs/git/clone.go
Comment threadinternal/git_clone_test.go Outdated
Comment threadinternal/git_clone_test.go Outdated
Comment threadinternal/git_clone_test.go Outdated
Comment threadlibs/git/clone.go Outdated
Comment threadlibs/git/clone_test.go Outdated
// case: invalid repository references
assert.Equal(t, "https://github.com/databricks/", expandUrl(""))
assert.Equal(t, "https://github.com//", expandUrl("/"))
assert.Equal(t, "https://github.com//abc/def", expandUrl("/abc/def"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These should return errors.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

removed the expand url function

@shreyas-goenkashreyas-goenkaJul 23, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

It's complex to have a simple and robust way to parse these URLs, across different protocols, git providers and operating systems. I decided to just support the repository name and the full URL to keep it simpler, with a simple rubrik, that ^[\w-\.]+$ without any / will be resolved against databricks

(see: https://stackoverflow.com/questions/59081778/rules-for-special-characters-in-github-repository-name)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note ^[\w-\.]+/[\w-\.]+$ is likely a valid regex that can be used as a capturing org name and repository name, but that can be added in as a followup

Comment threadlibs/git/clone.go
return args
}

func Clone(ctx context.Context, url, reference, targetPath string) error {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The argument suggests reference but the function above and test suggest it can only be a branch.

Which one applies?

@shreyas-goenkashreyas-goenkaJul 25, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The git clone --branch flag can take in both tags and branches.

source: https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---branchltnamegt

@shreyas-goenka
shreyas-goenka merged commit 8fdc0fe into mainJul 25, 2023
@shreyas-goenka
shreyas-goenka deleted the git-clone-clone branch July 25, 2023 13:36
@mgyuchtmgyucht mentioned this pull request Jul 27, 2023
github-merge-queueBot pushed a commit that referenced this pull request Jul 27, 2023
Breaking Change:
* Require include glob patterns to be explicitly defined
([#602](#602)).
Bundles:
* Add support for more SDK config options
([#587](#587)).
* Add template renderer for Databricks templates
([#589](#589)).
* Fix formatting in renderer.go
([#593](#593)).
* Fixed python wheel test
([#608](#608)).
* Auto detect Python wheel packages and infer build command
([#603](#603)).
* Added support for artifacts building for bundles
([#583](#583)).
* Add support for cloning repositories
([#544](#544)).
* Add regexp compile helper function for templates
([#601](#601)).
* Add unit test that raw strings are printed as is
([#599](#599)).
Internal:
* Fix tests under ./cmd/configure if DATABRICKS_TOKEN is set
([#605](#605)).
* Remove dependency on global state in generated commands
([#595](#595)).
* Remove dependency on global state for the root command
([#606](#606)).
* Add merge_group trigger for build
([#612](#612)).
* Added support for build command chaining and error on missing wheel
([#607](#607)).
* Add TestAcc prefix to filer test and fix any failing tests
([#611](#611)).
* Add url parse helper function for templates
([#600](#600)).
* Remove dependency on global state for remaining commands
([#613](#613)).
* Update CHANGELOG template
([#588](#588)).
denik pushed a commit that referenced this pull request May 20, 2026
## Changes
Adds support for cloning public and private github repositories for
databricks templates
## Tests
Integration tests
denik pushed a commit that referenced this pull request May 20, 2026
Breaking Change:
* Require include glob patterns to be explicitly defined
([#602](#602)).
Bundles:
* Add support for more SDK config options
([#587](#587)).
* Add template renderer for Databricks templates
([#589](#589)).
* Fix formatting in renderer.go
([#593](#593)).
* Fixed python wheel test
([#608](#608)).
* Auto detect Python wheel packages and infer build command
([#603](#603)).
* Added support for artifacts building for bundles
([#583](#583)).
* Add support for cloning repositories
([#544](#544)).
* Add regexp compile helper function for templates
([#601](#601)).
* Add unit test that raw strings are printed as is
([#599](#599)).
Internal:
* Fix tests under ./cmd/configure if DATABRICKS_TOKEN is set
([#605](#605)).
* Remove dependency on global state in generated commands
([#595](#595)).
* Remove dependency on global state for the root command
([#606](#606)).
* Add merge_group trigger for build
([#612](#612)).
* Added support for build command chaining and error on missing wheel
([#607](#607)).
* Add TestAcc prefix to filer test and fix any failing tests
([#611](#611)).
* Add url parse helper function for templates
([#600](#600)).
* Remove dependency on global state for remaining commands
([#613](#613)).
* Update CHANGELOG template
([#588](#588)).
Sign up for freeto 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

@shreyas-goenka@pietern@andrewnester