Add file download capabilities for GitHub and Gitlab providers #80

josecordaz · 2021-05-05T22:43:53Z

Description

Added new .Files().Get(ctx,PATH,BRANCH) function to download files on Github and Gitlab

Test results

Github tests output:

=== RUN   Test_clientOptions_getTransportChain
--- PASS: Test_clientOptions_getTransportChain (0.00s)
=== RUN   Test_clientOptions_getTransportChain/all_roundtrippers
    --- PASS: Test_clientOptions_getTransportChain/all_roundtrippers (0.00s)
=== RUN   Test_clientOptions_getTransportChain/only_pre_+_auth
    --- PASS: Test_clientOptions_getTransportChain/only_pre_+_auth (0.00s)
=== RUN   Test_clientOptions_getTransportChain/only_cache_+_auth
    --- PASS: Test_clientOptions_getTransportChain/only_cache_+_auth (0.00s)
=== RUN   Test_makeOptions
--- PASS: Test_makeOptions (0.00s)
=== RUN   Test_makeOptions/no_options
    --- PASS: Test_makeOptions/no_options (0.00s)
=== RUN   Test_makeOptions/WithDomain
    --- PASS: Test_makeOptions/WithDomain (0.00s)
=== RUN   Test_makeOptions/WithDomain,_empty
    --- PASS: Test_makeOptions/WithDomain,_empty (0.00s)
=== RUN   Test_makeOptions/WithDestructiveAPICalls
    --- PASS: Test_makeOptions/WithDestructiveAPICalls (0.00s)
=== RUN   Test_makeOptions/WithPreChainTransportHook
    --- PASS: Test_makeOptions/WithPreChainTransportHook (0.00s)
=== RUN   Test_makeOptions/WithPreChainTransportHook,_nil
    --- PASS: Test_makeOptions/WithPreChainTransportHook,_nil (0.00s)
=== RUN   Test_makeOptions/WithPostChainTransportHook
    --- PASS: Test_makeOptions/WithPostChainTransportHook (0.00s)
=== RUN   Test_makeOptions/WithPostChainTransportHook,_nil
    --- PASS: Test_makeOptions/WithPostChainTransportHook,_nil (0.00s)
=== RUN   Test_makeOptions/WithOAuth2Token
    --- PASS: Test_makeOptions/WithOAuth2Token (0.00s)
=== RUN   Test_makeOptions/WithOAuth2Token,_empty
    --- PASS: Test_makeOptions/WithOAuth2Token,_empty (0.00s)
=== RUN   Test_makeOptions/WithConditionalRequests
    --- PASS: Test_makeOptions/WithConditionalRequests (0.00s)
=== RUN   Test_makeOptions/WithConditionalRequests,_exclusive
    --- PASS: Test_makeOptions/WithConditionalRequests,_exclusive (0.00s)
=== RUN   TestProvider
Running Suite: GitHub Provider Suite
====================================
Random Seed: 1620254571
Will run 9 of 9 specs

• [SLOW TEST:6.703 seconds]
GitHub Provider
/Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/github/integration_test.go:127
  should list the available organizations the user has access to
  /Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/github/integration_test.go:167
------------------------------
••••••••
Ran 9 of 9 Specs in 19.563 seconds
SUCCESS! -- 9 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestProvider (19.56s)
=== RUN   Test_getPermissionFromMap
--- PASS: Test_getPermissionFromMap (0.00s)
=== RUN   Test_getPermissionFromMap/pull
    --- PASS: Test_getPermissionFromMap/pull (0.00s)
=== RUN   Test_getPermissionFromMap/push
    --- PASS: Test_getPermissionFromMap/push (0.00s)
=== RUN   Test_getPermissionFromMap/admin
    --- PASS: Test_getPermissionFromMap/admin (0.00s)
=== RUN   Test_getPermissionFromMap/none
    --- PASS: Test_getPermissionFromMap/none (0.00s)
=== RUN   Test_getPermissionFromMap/false_data
    --- PASS: Test_getPermissionFromMap/false_data (0.00s)
=== RUN   Test_getPermissionFromMap/not_all_specifed
    --- PASS: Test_getPermissionFromMap/not_all_specifed (0.00s)
=== RUN   Test_validateAPIObject
--- PASS: Test_validateAPIObject (0.00s)
=== RUN   Test_validateAPIObject/no_error_=>_nil
    --- PASS: Test_validateAPIObject/no_error_=>_nil (0.00s)
=== RUN   Test_validateAPIObject/one_error_=>_MultiError_&_InvalidServerData
    --- PASS: Test_validateAPIObject/one_error_=>_MultiError_&_InvalidServerData (0.00s)
=== RUN   Test_allPages
--- PASS: Test_allPages (0.00s)
=== RUN   Test_allPages/one_page_only,_no_error
    --- PASS: Test_allPages/one_page_only,_no_error (0.00s)
=== RUN   Test_allPages/two_pages,_no_error
    --- PASS: Test_allPages/two_pages,_no_error (0.00s)
=== RUN   Test_allPages/four_pages,_error_at_second
    --- PASS: Test_allPages/four_pages,_error_at_second (0.00s)
=== RUN   ExampleOrganizationsClient_Get
--- PASS: ExampleOrganizationsClient_Get (0.08s)
=== RUN   ExampleOrgRepositoriesClient_Get
--- PASS: ExampleOrgRepositoriesClient_Get (0.42s)
PASS

Gitlab tests output:

GOROOT=/usr/local/Cellar/go/1.16.3/libexec #gosetup
GOPATH=/Users/joseordaz/go #gosetup
/usr/local/Cellar/go/1.16.3/libexec/bin/go test -c -o /private/var/folders/24/xpk2zmbj2552mxs2nhzsq8t00000gn/T/___1go_test_github_com_fluxcd_go_git_providers_gitlab.test github.com/fluxcd/go-git-providers/gitlab #gosetup
/usr/local/Cellar/go/1.16.3/libexec/bin/go tool test2json -t /private/var/folders/24/xpk2zmbj2552mxs2nhzsq8t00000gn/T/___1go_test_github_com_fluxcd_go_git_providers_gitlab.test -test.v -test.paniconexit0
=== RUN   Test_clientOptions_getTransportChain
--- PASS: Test_clientOptions_getTransportChain (0.00s)
=== RUN   Test_clientOptions_getTransportChain/all_roundtrippers
    --- PASS: Test_clientOptions_getTransportChain/all_roundtrippers (0.00s)
=== RUN   Test_clientOptions_getTransportChain/only_pre_+_auth
    --- PASS: Test_clientOptions_getTransportChain/only_pre_+_auth (0.00s)
=== RUN   Test_clientOptions_getTransportChain/only_cache_+_auth
    --- PASS: Test_clientOptions_getTransportChain/only_cache_+_auth (0.00s)
=== RUN   Test_makeOptions
--- PASS: Test_makeOptions (0.00s)
=== RUN   Test_makeOptions/no_options
    --- PASS: Test_makeOptions/no_options (0.00s)
=== RUN   Test_makeOptions/WithDomain
    --- PASS: Test_makeOptions/WithDomain (0.00s)
=== RUN   Test_makeOptions/WithDomain,_empty
    --- PASS: Test_makeOptions/WithDomain,_empty (0.00s)
=== RUN   Test_makeOptions/WithDestructiveAPICalls
    --- PASS: Test_makeOptions/WithDestructiveAPICalls (0.00s)
=== RUN   Test_makeOptions/WithPreChainTransportHook
    --- PASS: Test_makeOptions/WithPreChainTransportHook (0.00s)
=== RUN   Test_makeOptions/WithPreChainTransportHook,_nil
    --- PASS: Test_makeOptions/WithPreChainTransportHook,_nil (0.00s)
=== RUN   Test_makeOptions/WithPostChainTransportHook
    --- PASS: Test_makeOptions/WithPostChainTransportHook (0.00s)
=== RUN   Test_makeOptions/WithPostChainTransportHook,_nil
    --- PASS: Test_makeOptions/WithPostChainTransportHook,_nil (0.00s)
=== RUN   Test_makeOptions/WithOAuth2Token
    --- PASS: Test_makeOptions/WithOAuth2Token (0.00s)
=== RUN   Test_makeOptions/WithOAuth2Token,_empty
    --- PASS: Test_makeOptions/WithOAuth2Token,_empty (0.00s)
=== RUN   Test_makeOptions/WithConditionalRequests
    --- PASS: Test_makeOptions/WithConditionalRequests (0.00s)
=== RUN   Test_makeOptions/WithConditionalRequests,_exclusive
    --- PASS: Test_makeOptions/WithConditionalRequests,_exclusive (0.00s)
=== RUN   Test_DomainVariations
--- PASS: Test_DomainVariations (0.00s)
=== RUN   Test_DomainVariations/gitlab.com_domain
    --- PASS: Test_DomainVariations/gitlab.com_domain (0.00s)
=== RUN   Test_DomainVariations/custom_domain_without_protocol
    --- PASS: Test_DomainVariations/custom_domain_without_protocol (0.00s)
=== RUN   Test_DomainVariations/custom_domain_with_https_protocol
    --- PASS: Test_DomainVariations/custom_domain_with_https_protocol (0.00s)
=== RUN   Test_DomainVariations/custom_domain_with_http_protocol
    --- PASS: Test_DomainVariations/custom_domain_with_http_protocol (0.00s)
=== RUN   TestProvider
Running Suite: GitLab Provider Suite
====================================
Random Seed: 1620253200
Will run 12 of 12 specs

• [SLOW TEST:12.681 seconds]
GitLab Provider
/Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/gitlab/integration_test.go:172
  should list the available organizations the user has access to
  /Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/gitlab/integration_test.go:259
------------------------------
•
------------------------------
• [SLOW TEST:11.738 seconds]
GitLab Provider
/Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/gitlab/integration_test.go:172
  should be possible to create a group project
  /Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/gitlab/integration_test.go:316
------------------------------
•
------------------------------
• [SLOW TEST:6.864 seconds]
GitLab Provider
/Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/gitlab/integration_test.go:172
  should update if the org repo already exists when reconciling
  /Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/gitlab/integration_test.go:362
------------------------------
• [SLOW TEST:78.690 seconds]
GitLab Provider
/Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/gitlab/integration_test.go:172
  should update teams with access and permissions when reconciling
  /Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/gitlab/integration_test.go:405
------------------------------
•
------------------------------
• [SLOW TEST:8.607 seconds]
GitLab Provider
/Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/gitlab/integration_test.go:172
  should be possible to create a user project
  /Users/joseordaz/go/src/github.com/josecordaz/go-git-providers/gitlab/integration_test.go:591
------------------------------
••••Deleting the user repo:  test-repo2-347
Deleting the org repo:  test-org-repo-381
Deleting the shared org repo:  test-shared-org-repo-617

Ran 12 of 12 Specs in 172.358 seconds
SUCCESS! -- 12 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestProvider (172.36s)
=== RUN   Test_getGitProviderPermission
--- PASS: Test_getGitProviderPermission (0.00s)
=== RUN   Test_getGitProviderPermission/pull
    --- PASS: Test_getGitProviderPermission/pull (0.00s)
=== RUN   Test_getGitProviderPermission/push
    --- PASS: Test_getGitProviderPermission/push (0.00s)
=== RUN   Test_getGitProviderPermission/admin
    --- PASS: Test_getGitProviderPermission/admin (0.00s)
=== RUN   Test_getGitProviderPermission/false_data
    --- PASS: Test_getGitProviderPermission/false_data (0.00s)
=== RUN   Test_getGitlabPermission
--- PASS: Test_getGitlabPermission (0.00s)
=== RUN   Test_getGitlabPermission/pull
    --- PASS: Test_getGitlabPermission/pull (0.00s)
=== RUN   Test_getGitlabPermission/push
    --- PASS: Test_getGitlabPermission/push (0.00s)
=== RUN   Test_getGitlabPermission/admin
    --- PASS: Test_getGitlabPermission/admin (0.00s)
=== RUN   Test_validateAPIObject
--- PASS: Test_validateAPIObject (0.00s)
=== RUN   Test_validateAPIObject/no_error_=>_nil
    --- PASS: Test_validateAPIObject/no_error_=>_nil (0.00s)
=== RUN   Test_validateAPIObject/one_error_=>_MultiError_&_InvalidServerData
    --- PASS: Test_validateAPIObject/one_error_=>_MultiError_&_InvalidServerData (0.00s)
=== RUN   Test_allGroupPages
--- PASS: Test_allGroupPages (0.00s)
=== RUN   Test_allGroupPages/one_page_only,_no_error
    --- PASS: Test_allGroupPages/one_page_only,_no_error (0.00s)
=== RUN   Test_allGroupPages/two_pages,_no_error
    --- PASS: Test_allGroupPages/two_pages,_no_error (0.00s)
=== RUN   Test_allGroupPages/four_pages,_error_at_second
    --- PASS: Test_allGroupPages/four_pages,_error_at_second (0.00s)
=== RUN   TestExampleOrganizationsClient_Get
Name: fluxcd-testing-public. Location: fluxcd-testing-public.--- PASS: TestExampleOrganizationsClient_Get (0.38s)
PASS

Process finished with the exit code 0
=== RUN   ExampleOrgRepositoriesClient_Get
--- PASS: ExampleOrgRepositoriesClient_Get (0.52s)

Fix: #81

Signed-off-by: Jose C Ordaz <josecordaz@gmail.com>

jrryjcksn

Very nice!

bboreham

I have several questions which are mostly superficial.
I haven't looked at the meat of the PR.

.github/ISSUE_TEMPLATE/bug_report.md

.github/ISSUE_TEMPLATE/config.yml

bboreham · 2021-05-06T17:39:27Z

gitprovider/types_repository.go

+// File contains high-level information about a file either add it to a commit or for content downloads
+type File struct {
 	// Path is path where this file is located.
 	// +required
 	Path *string `json:"path"`

+	// Name is path where this file is located.
+	//
+	Name *string `json:"name"`
+


Could you do this by adding a new struct rather than renaming the existing one and adding a field which is sometimes redundant?

bboreham · 2021-05-06T17:40:05Z

gitprovider/types_repository.go

 	// Path is path where this file is located.
 	// +required
 	Path *string `json:"path"`

+	// Name is path where this file is located.
+	//
+	Name *string `json:"name"`


Is Name the last component of the path? If not, when would it differ?

bboreham · 2021-05-06T17:42:59Z

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/ktrysmt/go-bitbucket v0.6.2
 	github.com/onsi/ginkgo v1.14.0
 	github.com/onsi/gomega v1.10.1
-	github.com/xanzy/go-gitlab v0.33.0
+	github.com/xanzy/go-gitlab v0.43.0


This change should go in its own commit, perhaps in its own PR.

that is in master already, maybe I did something wrong when I ran git merge master ? or the squash ? 🤔

bboreham · 2021-05-06T17:44:00Z

gitlab/client_repository_commit.go

+		filePath := *file.Path
+		fileContent := *file.Content
+		commitActions = append(commitActions, &gitlab.CommitActionOptions{
+			Action:   &fileAction,
+			FilePath: &filePath,
+			Content:  &fileContent,


This change seems unmotivated by the description of the PR.
Is it caused by the version change?

I'll add a description for this

stefanprodan

Please rebase with upstream, as there are files in this PR that don't belong, like the CI changes from #82

…o download-files

…iles

Signed-off-by: Simon Howe <footless@gmail.com>

codecov-commenter · 2022-01-13T08:48:08Z

Codecov Report

Merging #80 (483b3fc) into main (ddea418) will increase coverage by 0.10%.
The diff coverage is 68.42%.

@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
+ Coverage   59.79%   59.90%   +0.10%     
==========================================
  Files          77       80       +3     
  Lines        4052     4108      +56     
==========================================
+ Hits         2423     2461      +38     
- Misses       1117     1127      +10     
- Partials      512      520       +8

Impacted Files	Coverage Δ
github/client_repository_commit.go	`70.21% <ø> (-0.63%)`	⬇️
stash/client_repository_file.go	`0.00% <0.00%> (ø)`
github/client_repository_file.go	`56.52% <56.52%> (ø)`
gitlab/client_repository_file.go	`71.42% <71.42%> (ø)`
stash/resource_repository.go	`62.09% <75.00%> (+0.43%)`	⬆️
github/resource_repository.go	`82.88% <100.00%> (+0.63%)`	⬆️
gitlab/resource_repository.go	`78.21% <100.00%> (+0.89%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ddea418...483b3fc. Read the comment docs.

Signed-off-by: Simon Howe <footless@gmail.com>

foot · 2022-01-13T09:00:56Z

Merge history might be a bit stale so lets make sure to Squash and merge this one?

nikimanoledaki · 2022-01-17T10:35:43Z

+1 as this would be really really useful for the current work being done to implement gitops add profile!

stefanprodan

LGTM

Thanks @josecordaz

josecordaz marked this pull request as draft May 5, 2021 22:44

josecordaz marked this pull request as ready for review May 5, 2021 22:45

Have an option to get file's content

e2175f0

Signed-off-by: Jose C Ordaz <josecordaz@gmail.com>

jrryjcksn approved these changes May 6, 2021

View reviewed changes

bboreham reviewed May 6, 2021

View reviewed changes

stefanprodan requested changes May 6, 2021

View reviewed changes

josecordaz and others added 4 commits May 6, 2021 13:26

Merge branch 'main' of https://github.com/fluxcd/go-git-providers int…

2d20197

…o download-files

Merge remote-tracking branch 'origin/main' into josecordaz-download-f…

dec9bf3

…iles

Revert changes a bit

38601a5

Signed-off-by: Simon Howe <footless@gmail.com>

Adds stubs for stash

483b3fc

Signed-off-by: Simon Howe <footless@gmail.com>

lint it

5653ad9

Signed-off-by: Simon Howe <footless@gmail.com>

foot requested a review from stefanprodan January 13, 2022 09:00

stefanprodan changed the title ~~Have an option to get file's content~~ Add file download capabilities for GitHub and Gitlab providers Jan 17, 2022

stefanprodan added the enhancement New feature or request label Jan 17, 2022

stefanprodan approved these changes Jan 17, 2022

View reviewed changes

foot merged commit 2e907fb into fluxcd:main Jan 17, 2022

nikimanoledaki mentioned this pull request Jan 19, 2022

Add option to get content from all directories recursively #143

Open

nikimanoledaki mentioned this pull request Jan 31, 2022

Implement add profile to install a profile to a cluster weaveworks/weave-gitops#1360

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add file download capabilities for GitHub and Gitlab providers #80

Add file download capabilities for GitHub and Gitlab providers #80

josecordaz commented May 5, 2021 •

edited

jrryjcksn left a comment

bboreham left a comment

bboreham May 6, 2021

bboreham May 6, 2021

bboreham May 6, 2021

josecordaz May 6, 2021

bboreham May 6, 2021

josecordaz May 6, 2021

stefanprodan left a comment

codecov-commenter commented Jan 13, 2022

foot commented Jan 13, 2022

nikimanoledaki commented Jan 17, 2022

stefanprodan left a comment

Add file download capabilities for GitHub and Gitlab providers #80

Add file download capabilities for GitHub and Gitlab providers #80

Conversation

josecordaz commented May 5, 2021 • edited

Description

Test results

jrryjcksn left a comment

Choose a reason for hiding this comment

bboreham left a comment

Choose a reason for hiding this comment

bboreham May 6, 2021

Choose a reason for hiding this comment

bboreham May 6, 2021

Choose a reason for hiding this comment

bboreham May 6, 2021

Choose a reason for hiding this comment

josecordaz May 6, 2021

Choose a reason for hiding this comment

bboreham May 6, 2021

Choose a reason for hiding this comment

josecordaz May 6, 2021

Choose a reason for hiding this comment

stefanprodan left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jan 13, 2022

Codecov Report

foot commented Jan 13, 2022

nikimanoledaki commented Jan 17, 2022

stefanprodan left a comment

Choose a reason for hiding this comment

josecordaz commented May 5, 2021 •

edited