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

Rather than auto update certs, or a cron job. consider Rails.cache.fetch? #6

Open
pjr0001 opened this issue Jan 1, 2018 · 13 comments

Comments

@pjr0001
Copy link

pjr0001 commented Jan 1, 2018

I was messing around with the gem and it seems like it'd be pretty easy to use Rails.cache.fetch to pull in the new certs if they're expired.

I see a few pros and cons:

Pros:

  • no cron job, if the certificate ttl is expired, Rails.cache.fetch could just pull in the new certs and store them in one step
  • no cache stampeding: rails caching has a built in option called race_condition_ttl that could be used to prevent multiple requests from fetching the certs at the same time.
  • no redis dependency (will just use whatever cache store is being set in the app config, although i prefer redis as well)

Cons:

  • doesn't appear there is an explicit rails dependency right now, but this approach would introduce one.
@fschuindt
Copy link
Owner

Cool! I think you're right. I'm on a new year vacation trip at the moment. Soon as I get back home later this week I will perform a analysis on implementing it. Until there, I will leave it open as you and anyone else can give us more information about it.

I will keep this issue updated.
Thank you. 😃

@fschuindt
Copy link
Owner

Well it's quite a good time without improving it. I'm sorry. My current job is getting me a lot of time.

I'm going to be very happy If someone pull request it.
Just let me know any ongoing work on it.

Otherwise I will need some time before I implement it here.

@penguinwokrs
Copy link
Contributor

Hello.
I have a new proposal.
It seems good to abolish redis and use the cache with faraday middleware.

P.S. I am not good at English, so I'm sorry by bad english.

@fschuindt
Copy link
Owner

I'm kinda busy with other projects and a little bit away from Ruby. So I'm afraid I can not help much regarding this subject at the present moment.

@penguinwokrs If you or anyone else feel in the mood to proceed with any contributions, it would be just wonderful. You can count with all my support for any questions that may appear during changes. Feel free to contact me via email or by creating GitHub issues here.

That being said, I do think that removing Redis is indeed a good option. But let's try also to make it independent from the Rails environment. May be good for it to still support other Ruby applications and frameworks.

@penguinwokrs
Copy link
Contributor

@fschuindt Thank you for me contact.
I am willing to contribute.
Create an issue and start working.

I think this gem is wonderful.
I think it is a very good thing to be able to use it only with Ruby applications.

@namiwang
Copy link

namiwang commented Aug 1, 2019

@penguinwokrs @fschuindt

Hey guys! Are you still working on this? I'm working on implement this feature in my own project, would like to contribute to this gem.

@fschuindt
Copy link
Owner

@namiwang Hi there!

I don't hear from @penguinwokrs for a long time, I don't think there's anyone working on it.
Would be great to have that. 👍

@gowthamgts
Copy link

This is really a nice to have.

@namiwang
Copy link

Hey guys, sorry for the delay, already implemented the function in one of my private project. It's not actually a fork, just a few methods.

Obviously, the impact of using a Rails.cache.fetch would be the delay when there's a cache miss.

Personally I see that as an acceptable trade-off, but what do you guys think, an option to use cache or just drop the whole redis-cron solution?

BTW maybe we could just depend on ActiveSupport::Cache instead of the whole rails, never tested that, seems possible tho.

@fschuindt
Copy link
Owner

It's long since any activity here, maybe I'll close this. But for now I'll leave it open with enhancement and help wanted. Maybe someone will do it, it would be nice to remove Redis as dependency and bring ActiveSupport::Cache instead, without the whole Rails.

However, if adding Rails is the only way, I think we're better with using Redis.

Maybe I can work on this in the future.

@jdoconnor
Copy link

jdoconnor commented May 22, 2023

I'm happy to take this on. My idea is to make a new configuration block that would take any ActiveSupport::Cache object and look like

FirebaseIdToken.configure do |config|
 config.cache_store = ActiveSupport::Cache::MemoryStore.new(namespace: 'foobar')
 config.project_ids = ['your-firebase-project-id']
end

Any concerns?

@fschuindt
Copy link
Owner

@jdoconnor That looks great. I don't think there are any issues in going with that. Nice idea, btw.

@jdoconnor
Copy link

WIP: #43

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants