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 globbing support #38

Closed
wants to merge 11 commits into from
Closed

Add globbing support #38

wants to merge 11 commits into from

Conversation

crozzy
Copy link

@crozzy crozzy commented Apr 6, 2016

This is loosely based on the snakebite glob support.

@colinmarc
Copy link
Owner

Hi! is there any reason path.Match doesn't work? I use that in the command line client, which already supports globs (implementation here).

@crozzy
Copy link
Author

crozzy commented Apr 6, 2016

Hi! I guess I didn't think about using it just because I'm comparing 2 strings not paths. But path.Match does make more sense in the way it handles globs.

@crozzy
Copy link
Author

crozzy commented Apr 6, 2016

I actually started using this today and as the go os.FileInfo doesn't have a path type it made more sense in my use case to return a []string of matching paths.

@colinmarc colinmarc mentioned this pull request Apr 9, 2016
@@ -41,6 +41,11 @@ func New(address string) (*Client, error) {
return NewForUser(address, username)
}

// Return a New Client with auto config
func AutoConfigClient() (*Client, error) {
Copy link
Owner

Choose a reason for hiding this comment

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

Rather than a new method, let's just have this be default fallback behavior for New if an empty string is passed for the namenode.

@colinmarc
Copy link
Owner

Regarding globs: I don't really see the benefit to including globs in the library, since the command line client already supports them and they're really a bash thing. I'd cite that there's no equivalent method in the go os package. A user could easily use path.Match to implement something like you have here if they really want to.

I'm interested in this config loading stuff, though! Thanks so much for being willing to contribute, it really makes my day whenever there's a PR on this project.

Port int
}

type nameNodeError struct {
Copy link
Owner

Choose a reason for hiding this comment

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

There's no need for this to be a struct. it can just be var ErrUnresolvedNamenode = errors.New("Couldn't find a namenode address in any config.")

crozzy added a commit to crozzy/hdfs that referenced this pull request Apr 11, 2016
@crozzy crozzy mentioned this pull request Apr 11, 2016
@colinmarc colinmarc closed this Apr 11, 2016
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

Successfully merging this pull request may close these issues.

None yet

2 participants