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

blob: remove global registry for Open #931

Closed
zombiezen opened this issue Dec 11, 2018 · 5 comments
Closed

blob: remove global registry for Open #931

zombiezen opened this issue Dec 11, 2018 · 5 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@zombiezen
Copy link
Contributor

blob.Open currently operates from a global, mutable registry (populated with blob.Register). Go Cloud emphatically tries to avoid the pattern of having a global registry due to the confusion that can come in the face of conflicting keys and the difficulty of reasoning about side effects, especially when incorporating into a large application or when multiple incompatible versions are present. Avoiding global mutable state in favor of explicit construction is preferred.

Proposed Change

The blob package should grow a new type (called Opener below, but exact name TBD) that manages the mapping to different implementations. A reasonable default package can exist in the Go Cloud repository that imports GCS, S3, and Azure.

package blob

// An Opener returns buckets that map to pseudo-URLs.
type Opener struct {
  // ...
}

// NewOpener returns a new Opener that uses the given mapping of protocol schemes.
func NewOpener(schemes map[string]FromURLFunc) *Opener

// Open returns a bucket for the given URL based on its mapping.
func (o *Opener) Open(ctx context.Context, urlstr string) (*Bucket, error) {
}

and elsewhere:

package cloudbuckets

import (
  ".../azureblob"
  ".../gcsblob"
  ".../s3blob"
)

// Opener returns a *blob.Opener that maps URLs to common cloud providers.
func Opener() *blob.Opener {
  // ...
}

While this adds a bit more setup code, it:

  • Allows different *blob.Openers to be present in different parts of the application
  • Makes the connection between the Open call and the bucket much more explicit
  • Permits decoration or alteration of the FromURLFuncs being sent to the opener

/cc @vangent @shantuo

@zombiezen zombiezen added the enhancement New feature or request label Dec 11, 2018
zombiezen added a commit to zombiezen/go-cloud that referenced this issue Dec 11, 2018
@vangent
Copy link
Contributor

vangent commented Dec 11, 2018

Importantly: can you add code snippets showing how a user application would use this? Assume they want to pick and choose which providers they want to support (although choosing the "default" ones should be easier).

@vangent vangent added this to the Sprint 21 milestone Dec 13, 2018
@jba
Copy link
Contributor

jba commented Dec 13, 2018

Is it worth currying this? We could just have

blob.OpenURL(urlstr string, schemes map[string]FromURLFunc) (*Bucket, error)

@vangent
Copy link
Contributor

vangent commented Dec 17, 2018

On #981 (comment), @clausti suggested that maybe the scheme(s) that a provider registers for should be exposed as public constants for the package.

@zombiezen zombiezen added the in progress This is being actively worked on label Jan 8, 2019
@zombiezen
Copy link
Contributor Author

zombiezen commented Jan 8, 2019

One additional consideration that occurred to me while hacking on this (and this might be a separate issue): do we want to support being able to open readers and writers by URL? It seems like a natural extension.

EDIT: This is a separate issue. Opened #1101 to track.

@zombiezen
Copy link
Contributor Author

Obsoleted by design decisions in #1174.

@go-cloud-bot go-cloud-bot bot removed the in progress This is being actively worked on label Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants