Skip to content

Conversation

@jonathanedey
Copy link
Contributor

@jonathanedey jonathanedey commented Nov 19, 2024

Includes:

  • Auth
  • Installations / Instance ID
  • Messaging
  • Topic Management

This does not include:

  • App Check
  • Firestore
  • RTDB
  • Storage

@jonathanedey jonathanedey added the release:stage Stage a release candidate label Nov 19, 2024
jwks, err := keyfunc.Get(JWKSUrl, keyfunc.Options{
Ctx: ctx,
RefreshInterval: 6 * time.Hour,
RequestFactory: makeRequestFactory(conf),
Copy link
Contributor Author

@jonathanedey jonathanedey Nov 19, 2024

Choose a reason for hiding this comment

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

Only way to pass headers here is to pass it through a request factory. This is a little more involved, should I separate this change into it's own PR?

Copy link
Member

Choose a reason for hiding this comment

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

Since this request is just for the key server (we might not track the requests there anyways) let's skip it for now then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good

@jonathanedey jonathanedey marked this pull request as ready for review November 19, 2024 19:20
Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

LGTM
Added one comment on passing in the version to the helper function

}

// GetMetricsHeader constructs header value for metrics attribution
func GetMetricsHeader(sdkVersion string) string {
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to access the Version in here? That might be better than passing it in from all the APIs separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not possible, that would cause a dependency cycle. I'm not a fan of how this is structured either. I would prefer if we kept the Version value separately from firebase.go.

@jonathanedey jonathanedey merged commit 4ff0812 into dev Nov 21, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:stage Stage a release candidate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants