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

Add option to get content from all directories recursively #143

Open
nikimanoledaki opened this issue Jan 19, 2022 · 5 comments
Open

Add option to get content from all directories recursively #143

nikimanoledaki opened this issue Jan 19, 2022 · 5 comments

Comments

@nikimanoledaki
Copy link

nikimanoledaki commented Jan 19, 2022

The current implementation to GET a repository's content that was added in #80 does not get a directory tree recursively - it only returns a single, end subdirectory and its files. This is ok for now, but returning all directories recursively could be enabled easily by passing an option with RepositoryContentGetOptions, specifically here.

See documentation:

Alternatively, it would be nice to have the option to pass any RepositoryContentGetOptions :)

@nikimanoledaki nikimanoledaki changed the title Add option to get content from all directories recursively [Nice to have] Add option to get content from all directories recursively Jan 19, 2022
@nikimanoledaki nikimanoledaki changed the title [Nice to have] Add option to get content from all directories recursively Add option to get content from all directories recursively Jan 19, 2022
@foot
Copy link
Collaborator

foot commented May 23, 2022

Current prototype looks like this:

https://github.com/fluxcd/go-git-providers/blob/main/gitprovider/client.go#L244

type FileClient interface {
	// GetFiles fetch files content from specific path and branch
	Get(ctx context.Context, path, branch string) ([]*CommitFile, error)
}

So we could either add another method GetFileRecursively / GetWithOptions or add an extra arg ...Options to keep things backwards compatible?

@souleb
Copy link
Member

souleb commented May 23, 2022

I would prefer having a functional option... if I'm being asked 😄

@foot
Copy link
Collaborator

foot commented May 23, 2022

if I'm being asked 😄

'course you are :D,

Oh yeah an options thingy ay. Maybe

type Options struct {
	Recursive bool
}

type Option func(*Options)

func WithRecursive(bool value) Option {
	return func(o *Options) {
		o.Recursive = value
	}
}

Get(ctx context.Context, path, branch string, optFns ...Option) ([]*CommitFile, error)

And call it?

Get(ctx, "./clusters/my-cluster", "main", gitprovider.WithRecursive(true))

I'm not soo familiar w/ this pattern so if you have a good idea what the interfaces could look like please feel free to scribble something else!

@foot
Copy link
Collaborator

foot commented May 23, 2022

@foot
Copy link
Collaborator

foot commented May 24, 2022

On tackling:

  • Just run the github tests GIT_PROVIDER_ORGANIZATION=foo-org GIT_PROVIDER_USER=foo go test -v ./github/...
  • This looks like a good test case we could adapt
    It("should be possible to download files from path and branch specified", func() {
    to check for files flat and recursive.

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

No branches or pull requests

3 participants