Skip to content
This repository has been archived by the owner on Jul 15, 2024. It is now read-only.

Implement Git Files Discovery #29

Closed
OmerKahani opened this issue Sep 2, 2020 · 4 comments · Fixed by #45
Closed

Implement Git Files Discovery #29

OmerKahani opened this issue Sep 2, 2020 · 4 comments · Fixed by #45

Comments

@OmerKahani
Copy link
Contributor

Implement this example https://github.com/argoproj-labs/applicationset/blob/master/examples/git-files-discovery.yaml

Relate to #28

Might requires changes to the repo service

@rtnpro
Copy link
Contributor

rtnpro commented Sep 2, 2020

I want to work on this. However, there's some clarifications I need on this spec before proceeding.

Should we implement the file path filter regex as specified here: **/config.json? Contrary to the GitDirectory generator implementation, the directory path filter regex, e.g., addons/* is a valid filter working with Golang's path library.

But the regex used in the example here won't work as is to filter file paths using the path library. Examples

image

However, using a pattern like */*/*/config.json works perfectly with path.Match for paths specified in the example here.

image

I think that using a valid regex pattern that's directly compatible with path.Match(...) rather than coming up with a new non standard pattern like **/config.json would make more sense. Also, */*/*/config.json explicitly mentions at which level of the directory tree to look for the config.json file.

Thoughts? cc @OmerKahani @dgoodwin @xianlubird

@OmerKahani
Copy link
Contributor Author

I used path.Match in #7

@rtnpro
Copy link
Contributor

rtnpro commented Sep 9, 2020

I have a few more queries.

  • Are Git directory generator and Git file generator mutually exclusive? (Seems so to me from the examples)
  • What are the allowed patterns/variations for path in git file generator? **/config.json will force to look for cluster configs in the entire repo. On the other hand, cluster-configs/**/config.json would be able limit search for configs to specific paths. What all patterns do we plan to support for path for file discovery?

cc @OmerKahani @xianlubird

@OmerKahani
Copy link
Contributor Author

I think we can start with them been mutually exclusive.

About the paths, both examples are good, and we should probably support both. I don't think we can map all the patterns in advance, we should structure the code so adding a new pattern is easy and can be tested with a unit test.
I will recommend starting with one or two patterns, get the PR merged, and then start to think about other patterns.

Hope this is helpful

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants