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

[Proposal] Pass additional argument to adapters #1325

Merged
merged 1 commit into from
Mar 18, 2016
Merged

Conversation

particlebanana
Copy link
Contributor

Right now there doesn't exist a way to pass additional metadata for a query down to the adapter. This could be helpful when building custom adapters that need handle things like passing connections around or adding information for multi-tenancy.

This PR adds that ability in a temporary format. For now you can tack on an additional argument to your queries after the callback or use the .meta() key in the deferred object builder to pass any arbitrary data down to an adapter function.

Yes this breaks the common node contract of the callback as the last argument and it's weird. I know this isn't a great longterm solution. If you don't use the extra argument everything will work as expected though. The big plus is it gives us this functionality now without breaking every single one of the community adapters.

In the major version bump to 1.0.0 we will retool the function signatures to take an (options, cb) signature so that we can add non-breaking changes in the future.

Some examples of usage:

// Callback Style
var meta = function() {
  return 'Hello, I\'m a meta function';
};

User.find({ name: 'tester' }, function(err, users) {
    // some logic
}, meta);
// Deferred Style
var meta = function() {
  return 'Hello, I\'m a meta function';
};

User.find({ name: 'tester' })
.meta(meta)
.exec(function(err, users) {
 // some logic
});

To access the meta value in the adapter you would build a function like so:

find: function(connectionName, table, options, cb, meta) {
  if(typeof meta === 'function') {
    console.log(meta());
  }
}

For some more background on this see #1307

@sailsbot
Copy link

Hi @particlebanana! It looks like your pull request title doesn’t quite conform to our guidelines. Please edit the title so that it starts with [proposal], [patch], [fixes #], or [implements #]. Once you've fixed it, post a comment below (e.g. "ok, fixed!") and we'll take a look!

@Nishchit14
Copy link

👍 wow..
some hope for multi-tenancy.
Any plan for patch release ?

@particlebanana
Copy link
Contributor Author

@Nishchit14 looking at getting this and the projections pr published as 0.12 today.

particlebanana added a commit that referenced this pull request Mar 18, 2016
[Proposal] Pass additional argument to adapters
@particlebanana particlebanana merged commit 8ad46af into master Mar 18, 2016
@particlebanana particlebanana deleted the meta-argument branch March 18, 2016 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants