Re: Style of messages for checkArgument/checkNotNull in IOs

Huge +1 to the checkArgument(username != null, ...) style. A note on validate(), aren't we trying to remove pipeline options from PTransforms altogether (and, in addition, how does this even work with the Runner API and cross-language transforms). On Thu, Aug 10, 2017 at 4:59 PM, Eugene

Re: Style of messages for checkArgument/checkNotNull in IOs

https://beam.apache.org/contribute/ptransform-style-guide/#validation now includes the new guidance. It also includes updated guidance on what to put in expand() vs. validate() (TL;DR: validate() is almost always unnecessary. Put almost all validation in expand()) On Fri, Jul 28, 2017 at 11:56

Re: Style of messages for checkArgument/checkNotNull in IOs

So here's an easy solution: our own checkNotNull that throws InvalidArgumentException with a good error message. The error message can then be templated to allow terse invocations with just the method and parameter. Unsure why I didn't go for this straightaway. On Fri, Jul 28, 2017 at 11:43 AM,

Re: Style of messages for checkArgument/checkNotNull in IOs

Yuck. I think that checkNotNull throwing a NPE is a very poor design choice from the author of checkNotNull. On Fri, Jul 28, 2017 at 11:29 AM, Kenneth Knowles wrote: > As to the choice of check method: > > - checkArgument throws an InvalidArgumentException, which is

Re: Style of messages for checkArgument/checkNotNull in IOs

(I don't think changing messages in existing IOs is a high priority, btw - just a nice-to-have, as long as we have guidance for future IOs) On Fri, Jul 28, 2017 at 11:37 AM Eugene Kirpichov wrote: > Okay, so then let's change guidance to: > - Always use checkArgument to

Re: Style of messages for checkArgument/checkNotNull in IOs

Okay, so then let's change guidance to: - Always use checkArgument to check stuff that the user supplied - Use style 1 for messages on the checkArgument ? On Fri, Jul 28, 2017 at 11:31 AM Jean-Baptiste Onofré wrote: > Agree > > Regards > JB > > On Jul 28, 2017, 20:22, at

Re: Style of messages for checkArgument/checkNotNull in IOs

Agree Regards JB On Jul 28, 2017, 20:22, at 20:22, Thomas Groh wrote: >I'm in favor of the wording in the style of the first: it's an >immediately >actionable message that will have an associated stack trace, but will >provide the parameter in plaintext so the caller

Re: Style of messages for checkArgument/checkNotNull in IOs

Hi I'm a bit concerned because it has been discussed in a Jira and we discussed especially about this with Kenn and I. It dates from the incubation period. I changes most of the IOs I did for style 2 whereas I started with style 1. I'm ready to change again on the IOs but we agreed that

Re: Style of messages for checkArgument/checkNotNull in IOs

On Fri, Jul 28, 2017 at 11:21 AM, Thomas Groh wrote: > I'm in favor of the wording in the style of the first: it's an immediately > actionable message that will have an associated stack trace, but will > provide the parameter in plaintext so the caller doesn't have to

Re: Style of messages for checkArgument/checkNotNull in IOs

As to the choice of check method: - checkArgument throws an InvalidArgumentException, which is clearly in "your fault" category, a la HTTP 4xx - checkNotNull throws an NPE, which is usually a "my fault" exception, a la HTTP 5xx. The docs on NPE are not clear on blame, and this is a bug in the

Re: Style of messages for checkArgument/checkNotNull in IOs

I'm in favor of the wording in the style of the first: it's an immediately actionable message that will have an associated stack trace, but will provide the parameter in plaintext so the caller doesn't have to dig through the invoked code, they can just look at the documentation. I've recently

Style of messages for checkArgument/checkNotNull in IOs

Hey all, I think this has been discussed before on a JIRA issue but I can't find it, so raising again on the mailing list. Various IO (and non-IO) transforms validate their builder parameters using Preconditions.checkArgument/checkNotNull, and use different styles for error messages. There are 2