-
Notifications
You must be signed in to change notification settings - Fork 10
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: key not from file & other improvements #6
base: main
Are you sure you want to change the base?
Conversation
Looking good! Thank you for this. Shall we merge once your PR in gauth-rs is merged and published? We might also want this fork published, but I haven't done that before and I'm not sure about a proper crate name... I'm also not particularly comfortable with being the only maintainer or owner :) |
Yeah let's wait for the gauth-rs PR to be merged before merging this. I'm working on a few more tweaks/fixes and I'd also like to test all this out in prod on my end first to make sure everything is solid. I think we should also change the crate version from 1.0.0 to 0.10.0 since this PR makes breaking changes, and there could be more breaking changes going forward. Not sure it's a good idea to commit to a stable API quite yet. Regarding publishing the fork, it's possible you could get ownership of the crate name from the previous maintainer since that repo appears to no longer be maintained and the current version of FCM it uses will no longer work past June. Alternatively I'm happy to help maintain; my company also maintains the a2 library for APNs. |
Could you also have a look at #10? The |
It doesn't look like that PR provides a way to get a service account key from a string, also looks like there will be a lot of merge conflicts here. Perhaps we should drop gauth and switch to yup-oauth2? |
Yeah, that was also what I was thinking, but wanted to check with you because of this PR. |
This adds support for reading the service account key from a
ServiceAccountKey
rather than the default using thedotenv
variable. Also:dotenv
dependency and functionality forClient::new()
.new()
now returns aResult
since loading the key is performed immediately rather than later. RemoveDefault
implementation since it doesn't support errorsClientBuilder
pattern which enables more complex constructors such as allowing to provide a custom reqwest HTTP clientclient::response
to enable callers to match on the error variants. Re-exportgauth
to enable access toServiceAccountKey
project_id
fromServiceAccountKey
rather than double-parsing the value.pool_max_idle_per_host(usize::MAX)
configuration on reqwest since this is the default.json()
helper from reqwest to construct JSON request bodies over doing this by hand with.body()
,Content-Type
header, and serdeResolves #5
Depends on makarski/gauth-rs#29
Example usage in the project I'm working on: reown-com/push-server#316