search

stephenplusplus / google-auto-auth

Making it as easy as possible to authenticate a Google API request
MIT License
34 stars 9 forks source link

cache authClient across concurrent requests #11

Closed ofrobots closed 7 years ago

ofrobots commented 7 years ago

If multiple calls to getAuthClient are made concurrently before we can initialize the authClient, we end up with multiple authClient objects initialized in parallel. Use a promise as a better cache to avoid unnecessary work.

stephenplusplus commented 7 years ago

Thank you for this! GCN actually uses the authClient property (possibly other modules as well, can't remember). Could you still export that when the promise is resolved?

ofrobots commented 7 years ago

Done. PTAL.

ofrobots commented 7 years ago

Eeek. Now with a .gitignore added, and node_modules/ removed :smile:.

stephenplusplus commented 7 years ago

Haha, I was going to say, reviewing 2,728 files was gonna take a few minutes.

stephenplusplus commented 7 years ago

You should make one of these: https://help.github.com/articles/ignoring-files/#create-a-global-gitignore

ofrobots commented 7 years ago

Thanks for the tip about .gitignore_global. That is certainly very useful.

As a follow-on, I think we should do similar treatment to other async initialization done in this module. For example getEnvironment may make concurrent requests to the metadata service.

I found these through a somewhat contrived benchmark that attempts to write 10k Log Entries from logging. That is enough to stall my macbook because we end up making quite a few concurrent network requests all for the exact same thing.

stephenplusplus commented 7 years ago

Sounds good to me. I'm just going over this PR for a style treatment, then I'll merge in and we are free for future async management.

stephenplusplus commented 7 years ago

I pushed said changes-- can you take a look and make sure functionality was retained?