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

Pluggable credentials #149

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Pluggable credentials #149

wants to merge 21 commits into from

Conversation

jeremybarnes
Copy link
Contributor

This branch refactors the credentials handling code in S3 to provide:

  • A base infrastructure for pluggable credential providers
  • The S3 code now uses the base infrastructure
  • Existing credential registration functions add them into the base infrastructure

The idea here is to decouple credentials from the code that makes use of them, and allow for scenarios such as a daemon that provides short-lived credentials on demand or instance-based credentials on Amazon.

I'm primarily looking for comments on the design; please ignore the unfortunate noise in the early commits.

Adds an extra boolean parameter to the log category that allows it to be turned off by default, to support the 'recompile with verbose logging' use-case.  This could be combined at a later stage with the ability to modify logging via environment variables or a SHM segment and a signal.
This change adds a mustSucceed flag to the kill() and signal() functions in Runner, which
allows them to be called unconditionally.  This is especially useful in shutdowns, where we
don't want an exception if we tried to stop it but it was already stopped.  The default
behaviour remains the same: throw an exception.  They will also return whether they
succeded or not.

TRIVIAL because semantics of any existing calls have not changed.
This makes it possible to avoid segmentation faults in the case where the remote end drops the connection.
Previously it would lead to the connection object being deleted and a dangling pointer.

Mostly relevant to long-running connections that are handled asynchronously.  In the synchronous case the
connection is locked for the duration and recycled afterwards, so it can't cause a problem.
@wsourdeau
Copy link
Contributor

(quoted)
me: about the design for your credentials branch, I don't know what to think yet as I haven't had the time to look at it deeply. But I think the decoupling effort is a good thing in itself in any case. About s3, I would even go as far as decoupling buckets from credentials: s3Api instances could be instantiated with a credential objet right away.

response from @jeremybarnes :
I have basically done what you say, and removed the concept of having credentials for buckets

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 this pull request may close these issues.

2 participants