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

implemented embed client context #56

Closed
wants to merge 4 commits into from
Closed

implemented embed client context #56

wants to merge 4 commits into from

Conversation

awskii
Copy link
Contributor

@awskii awskii commented Jan 10, 2018

Hi.

Sometimes we need to store some metadata with client context, but there are no right way to do it. So i think it would be useful for any user of the library.

@fclairamb
Copy link
Owner

I don't think it's a good move:

  • It can be implemented on the client driver and you can now override the client's logger and add the additional parameter.
  • It makes the API more complex: for every call we add to the API we have to update all the existing implementations. By goal is to do the opposite: Avoid to implement things that might not be necessary to everyone and can be implemented at another level.
  • The standard context is usually performed for long synchronous calls. Here it would be used to store the FTP session context. It doesn't look like a good usage of it.

In other words if you take the sample implementation, you could have:

type ClientDriver struct {
	BaseDir string                 // Base directory from which to server file
	Context map[string]interface{} // Context
}
[...]
func (driver *MainDriver) AuthUser(cc server.ClientContext, user, pass string) (server.ClientHandlingDriver, error) {

	for _, act := range driver.config.Users {
		if act.User == user && act.Pass == pass {
			// If we are authenticated, we can return a client driver containing *our* basedir
			baseDir := driver.BaseDir + string(os.PathSeparator) + act.Dir
			os.MkdirAll(baseDir, 0777)
			return &ClientDriver{
				BaseDir: baseDir,
				Context: make(map[string]interface{}),
			}, nil
		}
	}

	return nil, fmt.Errorf("could not authenticate you")
}

@awskii
Copy link
Contributor Author

awskii commented Jan 16, 2018

You can't pass ClientHandlingDriver into UserLeft method, for example. Assume, i want to drop some cache if user has left, and the only way i have - use ClientContext. But i can't store some user data e.g. UUID in that context.

Correct me if I'm wrong.

@awskii awskii closed this Feb 25, 2018
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