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

Add callbacks on #create and #switch! #307

Merged
merged 1 commit into from
Jun 6, 2016

Conversation

cbeer
Copy link
Contributor

@cbeer cbeer commented Apr 18, 2016

One additional use case we have for multitenant applications is being able to switch additional connections to external resources (say, a search index connection or a storage bucket). We're currently triggering this logic through some hooks in the controller and/or job, but pushing it down into apartment would give us more confidence that everything is getting appropriately switched.

@cbeer cbeer force-pushed the callbacks branch 2 times, most recently from e367d85 to 43b9075 Compare April 18, 2016 23:22
@@ -80,10 +85,12 @@ def drop(tenant)
# @param {String} tenant name
#
def switch!(tenant = nil)
return reset if tenant.nil?
run_callbacks :switch do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't you want both switch! and switch (below) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. #switch uses #switch! internally, so I think #switch! is sufficient, unless we want to distinguish callbacks for #switch vs #switch!. I'm not sure it'd make a difference to what I'm trying to do, though. Thoughts?

Note, though, a call to #switch will run the callbacks twice, once when it switches into the new tenant and once when it reverts back to the previous one. This behavior is useful for my purposes, but could be unexpected without knowing the internals of #switch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with it being called twice on switch, just wasn't sure if this is what you'd expect.

@mjgiarlo
Copy link

mjgiarlo commented Jun 3, 2016

Would love to see this PR merged, and it looks like there are no objections or suggested changes. Anything holding this up? @bradrobertson ?

@bradrobertson bradrobertson merged commit a8006f9 into influitive:development Jun 6, 2016
@cbeer cbeer deleted the callbacks branch June 6, 2016 13:58
@mjgiarlo
Copy link

mjgiarlo commented Jun 8, 2016

Thanks, @bradrobertson! When do you think the next release (1.2.0?) might be cut?

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.

3 participants