Extend parameter checking support to Shepherd key mappings

Question

Extend parameter checking support to Shepherd key mappings

Closed this issue 11 years ago · 13 comments

When enforceMatchingParams is set, also check Shepherd key mappings ('x-forId', 'params.x', etc.).

Answer 1 · 2013-06-12T22:01:09.000Z

@nicks: So I'm not exactly clear on when the parameter checker would kick in here. I get that Shepherd interprets the node names x-forId and params.x as mapping to the argument x. But I'm less clear exactly what that looks like in practice. So would it be something like:

function byXY (x, y) {}

graph.add('z-byXY', byXY, ['x-byId', 'params.y'])

And the above should validate? Or does this arise in a different context? Thanks!

Answer 2 · 2013-06-12T22:03:35.000Z

yep. here's a good example:
https://github.com/Obvious/medium2/pull/5909

On Wed, Jun 12, 2013 at 6:01 PM, Kyle Hardgrave notifications@github.comwrote:

@nicks https://github.com/nicks: So I'm not exactly clear on when the
parameter checker would kick in here. I get that Shepherd interprets the
node names x-forId and params.x as mapping to the argument x. But I'm
less clear exactly what that looks like in practice. So would it be
something like:

function byXY (x, y) {}
graph.add('z-byXY', byXY, ['x-byId', 'params.y'])

And the above should validate? Or does this arise in a different context?
Thanks!

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/54#issuecomment-19358672
.

Answer 3 · 2013-06-12T22:20:30.000Z

So in the example you provided with addEmailToQueue, everything is an
already-added node that is passed as arguments except the last one, right?
So how should the checker handle this — does it need to distinguish globals
and ignore those and only check that the other params match?

On Wed, Jun 12, 2013 at 3:03 PM, Nick Santos notifications@github.comwrote:

yep. here's a good example:
https://github.com/Obvious/medium2/pull/5909

On Wed, Jun 12, 2013 at 6:01 PM, Kyle Hardgrave notifications@github.comwrote:

@nicks https://github.com/nicks: So I'm not exactly clear on when the
parameter checker would kick in here. I get that Shepherd interprets the
node names x-forId and params.x as mapping to the argument x. But I'm
less clear exactly what that looks like in practice. So would it be
something like:

function byXY (x, y) {}
graph.add('z-byXY', byXY, ['x-byId', 'params.y'])

And the above should validate? Or does this arise in a different
context?
Thanks!

—
Reply to this email directly or view it on GitHub<
https://github.com/Obvious/shepherd/issues/54#issuecomment-19358672>
.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/54#issuecomment-19358791
.

Answer 4 · 2013-06-12T22:22:45.000Z

i honestly have no idea what you're asking. can you give an example?

On Wed, Jun 12, 2013 at 6:20 PM, Kyle Hardgrave notifications@github.comwrote:

So in the example you provided with addEmailToQueue, everything is an
already-added node that is passed as arguments except the last one, right?
So how should the checker handle this — does it need to distinguish
globals
and ignore those and only check that the other params match?

On Wed, Jun 12, 2013 at 3:03 PM, Nick Santos notifications@github.comwrote:

yep. here's a good example:
https://github.com/Obvious/medium2/pull/5909

On Wed, Jun 12, 2013 at 6:01 PM, Kyle Hardgrave <
notifications@github.com>wrote:

@nicks https://github.com/nicks: So I'm not exactly clear on when
the
parameter checker would kick in here. I get that Shepherd interprets
the
node names x-forId and params.x as mapping to the argument x. But I'm
less clear exactly what that looks like in practice. So would it be
something like:

function byXY (x, y) {}
graph.add('z-byXY', byXY, ['x-byId', 'params.y'])

And the above should validate? Or does this arise in a different
context?
Thanks!

—
Reply to this email directly or view it on GitHub<
https://github.com/Obvious/shepherd/issues/54#issuecomment-19358672>
.

—
Reply to this email directly or view it on GitHub<
https://github.com/Obvious/shepherd/issues/54#issuecomment-19358791>
.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/54#issuecomment-19359593
.

Answer 5 · 2013-06-12T22:24:23.000Z

oh, maybe the case you're worried about is:

graph.add('z-byXY', byXY, ['req.params.x'])
.builds('y-byId').using('id-default')

?

yes, that should validate.

On Wed, Jun 12, 2013 at 6:22 PM, Nick Santos nick@medium.com wrote:

i honestly have no idea what you're asking. can you give an example?

On Wed, Jun 12, 2013 at 6:20 PM, Kyle Hardgrave notifications@github.comwrote:

So in the example you provided with addEmailToQueue, everything is an
already-added node that is passed as arguments except the last one,
right?
So how should the checker handle this — does it need to distinguish
globals
and ignore those and only check that the other params match?

On Wed, Jun 12, 2013 at 3:03 PM, Nick Santos notifications@github.comwrote:

yep. here's a good example:
https://github.com/Obvious/medium2/pull/5909

On Wed, Jun 12, 2013 at 6:01 PM, Kyle Hardgrave <
notifications@github.com>wrote:

@nicks https://github.com/nicks: So I'm not exactly clear on when
the
parameter checker would kick in here. I get that Shepherd interprets
the
node names x-forId and params.x as mapping to the argument x. But I'm
less clear exactly what that looks like in practice. So would it be
something like:

function byXY (x, y) {}
graph.add('z-byXY', byXY, ['x-byId', 'params.y'])

And the above should validate? Or does this arise in a different
context?
Thanks!

—
Reply to this email directly or view it on GitHub<
https://github.com/Obvious/shepherd/issues/54#issuecomment-19358672>
.

—
Reply to this email directly or view it on GitHub<
https://github.com/Obvious/shepherd/issues/54#issuecomment-19358791>
.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/54#issuecomment-19359593
.

Answer 6 · 2013-06-12T22:37:44.000Z

I think that's what I'm asking, if that's analogous to what's going on in Obvious/medium2#5909. There, we have a function

function addEmailToQueue(cryptoHelper, templateHelper, htmlHelper, logger, stats, baseConfig, sesConfig, emailInfos, queue)

which gets added as

graph.add('bool-addEmailsToQueue', addEmailToQueue, ['helpers.Crypto', 'helpers.Template', 'shared.HtmlHelper', 'logger', 'stats', 'config.base', 'config.ses', 'emailInfos'])

Now all those arguments are actually preexisting nodes (right?) except emailInfos, and so they don't need to match the argument names of addEmailToQueue?

Answer 7 · 2013-06-13T03:17:31.000Z

Man, you interns are smart. How long have you been here? O_o

Answer 8 · 2013-06-13T12:49:24.000Z

yep, i think you got it (though the last part of your message looks
like it got cut off). so in the Obvious/medium2#5909 example, the
arguments of the function would have to be
(Crypto, Template, HtmlHelper, logger, stats, base, ses, emailInfos, sqsQueue)
which are slightly different than what we've got there, but i think
the consistency will help.

On Wed, Jun 12, 2013 at 11:17 PM, Sho Kuwamoto notifications@github.com wrote:

Man, you interns are smart. How long have you been here? O_o

—
Reply to this email directly or view it on GitHub.

Answer 9 · 2013-06-13T12:50:30.000Z

are you concerned that we might have trouble checking one of these?

On Thu, Jun 13, 2013 at 8:48 AM, Nick Santos nick@medium.com wrote:

yep, i think you got it (though the last part of your message looks
like it got cut off). so in the Obvious/medium2#5909 example, the
arguments of the function would have to be
(Crypto, Template, HtmlHelper, logger, stats, base, ses, emailInfos, sqsQueue)
which are slightly different than what we've got there, but i think
the consistency will help.

On Wed, Jun 12, 2013 at 11:17 PM, Sho Kuwamoto notifications@github.com wrote:

Man, you interns are smart. How long have you been here? O_o

—
Reply to this email directly or view it on GitHub.

Answer 10 · 2013-06-13T16:51:48.000Z

Ahh no, I wasn't sure if—because they're globals—they didn't have to match up exactly, but you're saying they should. That doesn't seem like it'll be a problem. Thanks, Nick, sorry for the slow understanding.

Answer 11 · 2013-06-13T17:03:42.000Z

ah, i think you've hit on the core idea though. globals do feel somehow
special, and i do think we eventually want special handling for them. for
example, we might have a world where we have:

graph.add('x-fromY', function ($db, y) {}, ['y'])

and shepherd would automatically figure out (from the argument name) that
db is a global key, and inject it.

On Thu, Jun 13, 2013 at 12:51 PM, Kyle Hardgrave
notifications@github.comwrote:

Ahh no, I wasn't sure if—because they're globals—they didn't have to
match up exactly, but you're saying they should. That doesn't seem like
it'll be a problem. Thanks, Nick, sorry for the slow understanding.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/54#issuecomment-19405452
.

Answer 12 · 2013-06-13T17:34:17.000Z

That'd be much nicer.

-- Dan

On Thu, Jun 13, 2013 at 10:03 AM, Nick Santos notifications@github.comwrote:

ah, i think you've hit on the core idea though. globals do feel somehow
special, and i do think we eventually want special handling for them. for
example, we might have a world where we have:

graph.add('x-fromY', function ($db, y) {}, ['y'])

and shepherd would automatically figure out (from the argument name) that
db is a global key, and inject it.

On Thu, Jun 13, 2013 at 12:51 PM, Kyle Hardgrave
notifications@github.comwrote:

Ahh no, I wasn't sure if—because they're globals—they didn't have to
match up exactly, but you're saying they should. That doesn't seem like
it'll be a problem. Thanks, Nick, sorry for the slow understanding.

—
Reply to this email directly or view it on GitHub<
https://github.com/Obvious/shepherd/issues/54#issuecomment-19405452>
.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/54#issuecomment-19406469
.

Answer 13 · 2013-06-13T17:35:08.000Z

btw, I'm not sure the db client should be global though. I want to track
number of DB calls per request and thought an easy way to do that would be
to make the DB client request scoped.

-- Dan

On Thu, Jun 13, 2013 at 10:33 AM, Dan Pupius dan@medium.com wrote:

That'd be much nicer.

-- Dan

On Thu, Jun 13, 2013 at 10:03 AM, Nick Santos notifications@github.comwrote:

ah, i think you've hit on the core idea though. globals do feel somehow
special, and i do think we eventually want special handling for them. for
example, we might have a world where we have:

graph.add('x-fromY', function ($db, y) {}, ['y'])

and shepherd would automatically figure out (from the argument name) that
db is a global key, and inject it.

On Thu, Jun 13, 2013 at 12:51 PM, Kyle Hardgrave
notifications@github.comwrote:

Ahh no, I wasn't sure if—because they're globals—they didn't have to
match up exactly, but you're saying they should. That doesn't seem like
it'll be a problem. Thanks, Nick, sorry for the slow understanding.

—
Reply to this email directly or view it on GitHub<
https://github.com/Obvious/shepherd/issues/54#issuecomment-19405452>
.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/54#issuecomment-19406469
.