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

Support for multi-tenancy #5

Open
chris13524 opened this issue Apr 9, 2024 · 4 comments · May be fixed by #6
Open

Support for multi-tenancy #5

chris13524 opened this issue Apr 9, 2024 · 4 comments · May be fixed by #6

Comments

@chris13524
Copy link

chris13524 commented Apr 9, 2024

This client depends on the filesystem and a specific dotenv configuration in order to configure the FCM client. My use case, however, requires working with a number of FCM credentials sourced from a database.

I think this depends on makarski/gauth-rs#28

I'm willing to contribute this support myself :)

@rj76
Copy link
Owner

rj76 commented Apr 10, 2024

Very useful to have this more flexible indeed!

We could extract the credentials part from fcm::client::Client and create a Credentials struct for this, which is then used by Client. This new struct allows a filename or string for the config and calls ServiceAccount::from_string or similar for your usecase.

@chris13524
Copy link
Author

chris13524 commented Apr 10, 2024

I was thinking very similar! We keep new() that defaults to ServiceAccount::from_file but then have a second method from_credentials() which accepts a string

This would also read the credentials file once on startup rather than twice for every message which is an important step for proper re-use of the Service Account access token.

@chris13524
Copy link
Author

Is support for hotswpping credential files on-disk an intentional feature? I'm thinking of removing this ability in my refactor as it would be simpler

@rj76
Copy link
Owner

rj76 commented Apr 18, 2024

I haven't given that much thought yet, no.

@chris13524 chris13524 linked a pull request Apr 18, 2024 that will close this issue
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 a pull request may close this issue.

3 participants
@rj76 @chris13524 and others