-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(pubsub): enable project autodetection and detect empty project #8168
Conversation
// NewClient creates a new PubSub client. It uses a default configuration. | ||
func NewClient(ctx context.Context, projectID string, opts ...option.ClientOption) (c *Client, err error) { | ||
return NewClientWithConfig(ctx, projectID, nil, opts...) | ||
} | ||
|
||
// NewClientWithConfig creates a new PubSub client. | ||
func NewClientWithConfig(ctx context.Context, projectID string, config *ClientConfig, opts ...option.ClientOption) (c *Client, err error) { | ||
if projectID == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hongalex doesn't having this check early on break the opts.WithoutAuthentication
case since it now requires you to pass a dummy projectID to instantiate the clients.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a breaking change that warrants a major version bump instead of a minor bump?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this may be backwards incompatible, but falls under the realm of a bug fix and doesn't warrant a major version bump. Passing in an empty string to the client is unsupported behavior and should have resulted in an error, even when using unauthenticated. The project string is used to construct paths like "projects/my-project/topics/my-topic" and creating resources like "projects//topics/my-topic" is undefined behavior.
Allows autodetection of project ID when the sentinel value (DetectProjectID) is passed in to
pubsub.NewClient
. This uses thedetect
, similar to BigQuery in #4312. Similar to Firestore in #4299, empty project IDs will now return an error.Also exposes a new method in pubsub client
client.Project()
which returns the project ID.Retrieving information about the topic's (and subscription) project ID can be done by
topic.Config
, followed by callingconfig.String()
.Addresses pubsub portion in #1294
Fixes #3828