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

MANTA-2781 mackerel: deliver-access does not handle backpressure properly #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joyent-automation
Copy link

MANTA-2781 mackerel: deliver-access does not handle backpressure properly

This PR was migrated-from-gerrit, https://cr.joyent.us/#/c/1599/.
The raw archive of this CR is here.
See MANTA-4594 for info on Joyent Eng's migration from Gerrit.

CR discussion

@jclulow commented at 2017-03-01T22:55:46

Topic set to MANTA-2781

@davepacheco commented at 2017-03-10T21:32:25

Patch Set 2: Code-Review+1

(4 comments)

@jclulow commented at 2017-03-16T19:12:51

Patch Set 2:

(4 comments)

Patch Set 2 code comments
assets/lib/deliver-access.js#1 @davepacheco

D: (not related to this change)

assets/lib/deliver-access.js#1 @jclulow

Sad face indeed.

assets/lib/deliver-access.js#207 @davepacheco

It'd be nice to have a comment about the input objects and output objects for this stream and what it does.

assets/lib/deliver-access.js#207 @jclulow

Done

assets/lib/deliver-access.js#318 @davepacheco

Could use a vasync barrier here.

assets/lib/deliver-access.js#318 @jclulow

That's true, but I think I'll punt on it for now. If we keep this code as part of Metering v2, I'll address it then.

package.json#16 @davepacheco

I'd suggest eliding the "^".

package.json#16 @jclulow

Done

@davepacheco commented at 2017-03-16T20:38:21

Patch Set 3: Code-Review+1

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.

None yet

2 participants