Re: Precomit broken due to style violation. Why are failures getting past precommit?

2019-01-29 Thread Scott Wegner
This is great, and exactly solves the problem we were having. FindBugs will
fail the compilation, but test flakes from another subproject can
potentially mask the failures.

On Tue, Jan 29, 2019, 3:56 PM Michael Luckey  As currently we miss prominent exposures of check style warnings, I added
> another post build action to publish check style and findbugs warnings. [1]
>
> This will lead to
> - show check style/findbugs status right on the status page
>
> - links to followup pages with some graphs
>
> This could help to not hide those warnings. Wdyt?
>
> Of course, if we find this useful, we might extend to other precommits
> and/or other tools. E.g. findbugs might be useless anyway, as error prone
> will fail compile anyway?
>
> [1] https://github.com/apache/beam/pull/7666
>
> On 24. Jan 2019, at 17:56, Scott Wegner  wrote:
>
> Yes, you're correct that PR#7571 [1] had a checkstyle violation when
> merged. I didn't notice the checkstyle failure and I hit the merge button.
> Sorry about that.
>
> Here's where I went wrong:
> * The precommits showed one failing: Java. GitHub shows the status as a
> green check or red X on the head commit for a PR [2].
> * Opening the Jenkins link [3], it shows "Test Result (1 failure/ +-0)",
> and clicking on that shows the failing test was testMatchWatchForNewFiles
> [4]
> * I recognized this as BEAM-6491 [5] and decided not to block the PR since
> it should be unrelated. So I hit merge.
>
> What I didn't do was click on the Gradle Build Scan link [6], which shows
> that :beam-runners-direct-java:checkstyleMain failed as well.
>
> I take the blame for letting this in, and I'll follow-up to make sure it
> gets fixed. I also think that this is an easy mistake to make. Some ideas
> to decrease the chances:
>
> a) Split out checkstyle and other static analysis as a separate pre-commit
> from running tests. Because Jenkins reports the test report separately and
> more prominently, it's easy to miss other failures. We already split out
> Spotless and Rat, which also provides the value of giving a faster signal
> on those checks.
>
> b) Have a stronger policy about not merging when tests are red. I just
> checked and this is actually the policy already [7], but exceptions are
> regularly made for flaky or unrelated test failures (evidence: each red X
> on the master commit history [8]). Right now it's up to a human to make the
> call on, and us humans are prone to make mistakes. We could consider
> enforcing the policy and configure GitHub to require all checks passing
> before merge. This will make flaky tests more painful, though it will also
> provide a stronger incentive to fix the flaky tests.
>
> [1] https://github.com/apache/beam/pull/7571
> [2] https://github.com/apache/beam/pull/7571/commits
> [3] https://builds.apache.org/job/beam_PreCommit_Java_Phrase/577/
> [4]
> https://builds.apache.org/job/beam_PreCommit_Java_Phrase/577/testReport/
> [5] https://jira.apache.org/jira/browse/BEAM-6491
> [6] https://scans.gradle.com/s/s3wdusaicauee
> [7] https://beam.apache.org/contribute/precommit-policies/#pull-requests
> [8] https://github.com/apache/beam/commits/master
>
> On Wed, Jan 23, 2019 at 5:39 PM Alex Amato  wrote:
>
>> See: https://issues.apache.org/jira/browse/BEAM-6500
>>
>> I think that this PR introduced the issue. Though I am not sure how to
>> read the test status. It looks like its marked with an X for the postcommit
>> status, but presumably the precommit was okay even though java precommit
>> appears to be broken now? Is there any explanation as to how this PR got
>> through?
>> https://github.com/apache/beam/pull/7571
>> 
>>
>> Today there have been numerous issues with presubmit. I would just like
>> to understand if there is some underlying issue going on here missing some
>> checks when we merge. Any ideas why this keeps occurring?
>>
>> Thanks
>> Alex
>>
>
>
> --
>
>
>
>
> Got feedback? tinyurl.com/swegner-feedback
>
>
>


Re: [VOTE] Release 2.10.0, release candidate #1

2019-01-29 Thread Ahmet Altay
-1, I ran into a new blocking issue:
https://issues.apache.org/jira/browse/BEAM-6545

On Tue, Jan 29, 2019 at 4:08 PM Kenneth Knowles  wrote:

> I have done this in the least vulnerable way I can think of. I have filed
> https://issues.apache.org/jira/browse/BEAM-6544 as a blocker to fix the
> release process.
>
> Kenn
>
> On Tue, Jan 29, 2019 at 3:07 PM Kenneth Knowles  wrote:
>
>> Yes, the instructions for building the wheels includes inputting my ASF
>> credentials into Travis-CI. I've been trying to understand why and what I
>> can do instead.
>>
>> (The release guide says that the release script builds the binaries, but
>> from what I can tell it does not. This makes sense because the instructions
>> are highly manual too.)
>>
>> Kenn
>>
>> On Tue, Jan 29, 2019 at 12:38 AM Robert Bradshaw 
>> wrote:
>>
>>> The artifacts and signatures look good. But we're missing Python wheels.
>>>
>>>
>>> On Tue, Jan 29, 2019 at 6:08 AM Kenneth Knowles  wrote:
>>> >
>>> > Ah, I did not close the staging repository. Thanks for letting me
>>> know. Try now.
>>> >
>>> > Kenn
>>> >
>>> > On Mon, Jan 28, 2019 at 2:31 PM Ismaël Mejía 
>>> wrote:
>>> >>
>>> >> I think there is an issue, [4] does not open?
>>> >>
>>> >> On Mon, Jan 28, 2019 at 6:24 PM Kenneth Knowles 
>>> wrote:
>>> >> >
>>> >> > Hi everyone,
>>> >> >
>>> >> > Please review and vote on the release candidate #1 for the version
>>> 2.10.0, as follows:
>>> >> >
>>> >> > [ ] +1, Approve the release
>>> >> > [ ] -1, Do not approve the release (please provide specific
>>> comments)
>>> >> >
>>> >> > The complete staging area is available for your review, which
>>> includes:
>>> >> > * JIRA release notes [1],
>>> >> > * the official Apache source release to be deployed to
>>> dist.apache.org [2], which is signed with the key with fingerprint
>>> 6ED551A8AE02461C [3],
>>> >> > * all artifacts to be deployed to the Maven Central Repository [4],
>>> >> > * source code tag "v2.10.0-RC1" [5],
>>> >> > * website pull request listing the release [6] and publishing the
>>> API reference manual [7].
>>> >> > * Python artifacts are deployed along with the source release to
>>> the dist.apache.org [2].
>>> >> > * Validation sheet with a tab for 2.10.0 release to help with
>>> validation [7].
>>> >> >
>>> >> > The vote will be open for at least 72 hours. It is adopted by
>>> majority approval, with at least 3 PMC affirmative votes.
>>> >> >
>>> >> > Thanks,
>>> >> > Kenn
>>> >> >
>>> >> > [1]
>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12319527=12344540
>>> >> > [2] https://dist.apache.org/repos/dist/dev/beam/2.10.0/
>>> >> > [3] https://dist.apache.org/repos/dist/release/beam/KEYS
>>> >> > [4]
>>> https://repository.apache.org/content/repositories/orgapachebeam-1056/
>>> >> > [5] https://github.com/apache/beam/tree/v2.10.0-RC1
>>> >> > [6] https://github.com/apache/beam/pull/7651/files
>>> >> > [7] https://github.com/apache/beam-site/pull/585
>>> >> > [8]
>>> https://docs.google.com/spreadsheets/d/1qk-N5vjXvbcEk68GjbkSZTR8AGqyNUM-oLFo_ZXBpJw/edit#gid=2053422529
>>>
>>


Re: [VOTE] Release 2.10.0, release candidate #1

2019-01-29 Thread Kenneth Knowles
I have done this in the least vulnerable way I can think of. I have filed
https://issues.apache.org/jira/browse/BEAM-6544 as a blocker to fix the
release process.

Kenn

On Tue, Jan 29, 2019 at 3:07 PM Kenneth Knowles  wrote:

> Yes, the instructions for building the wheels includes inputting my ASF
> credentials into Travis-CI. I've been trying to understand why and what I
> can do instead.
>
> (The release guide says that the release script builds the binaries, but
> from what I can tell it does not. This makes sense because the instructions
> are highly manual too.)
>
> Kenn
>
> On Tue, Jan 29, 2019 at 12:38 AM Robert Bradshaw 
> wrote:
>
>> The artifacts and signatures look good. But we're missing Python wheels.
>>
>>
>> On Tue, Jan 29, 2019 at 6:08 AM Kenneth Knowles  wrote:
>> >
>> > Ah, I did not close the staging repository. Thanks for letting me know.
>> Try now.
>> >
>> > Kenn
>> >
>> > On Mon, Jan 28, 2019 at 2:31 PM Ismaël Mejía  wrote:
>> >>
>> >> I think there is an issue, [4] does not open?
>> >>
>> >> On Mon, Jan 28, 2019 at 6:24 PM Kenneth Knowles 
>> wrote:
>> >> >
>> >> > Hi everyone,
>> >> >
>> >> > Please review and vote on the release candidate #1 for the version
>> 2.10.0, as follows:
>> >> >
>> >> > [ ] +1, Approve the release
>> >> > [ ] -1, Do not approve the release (please provide specific comments)
>> >> >
>> >> > The complete staging area is available for your review, which
>> includes:
>> >> > * JIRA release notes [1],
>> >> > * the official Apache source release to be deployed to
>> dist.apache.org [2], which is signed with the key with fingerprint
>> 6ED551A8AE02461C [3],
>> >> > * all artifacts to be deployed to the Maven Central Repository [4],
>> >> > * source code tag "v2.10.0-RC1" [5],
>> >> > * website pull request listing the release [6] and publishing the
>> API reference manual [7].
>> >> > * Python artifacts are deployed along with the source release to the
>> dist.apache.org [2].
>> >> > * Validation sheet with a tab for 2.10.0 release to help with
>> validation [7].
>> >> >
>> >> > The vote will be open for at least 72 hours. It is adopted by
>> majority approval, with at least 3 PMC affirmative votes.
>> >> >
>> >> > Thanks,
>> >> > Kenn
>> >> >
>> >> > [1]
>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12319527=12344540
>> >> > [2] https://dist.apache.org/repos/dist/dev/beam/2.10.0/
>> >> > [3] https://dist.apache.org/repos/dist/release/beam/KEYS
>> >> > [4]
>> https://repository.apache.org/content/repositories/orgapachebeam-1056/
>> >> > [5] https://github.com/apache/beam/tree/v2.10.0-RC1
>> >> > [6] https://github.com/apache/beam/pull/7651/files
>> >> > [7] https://github.com/apache/beam-site/pull/585
>> >> > [8]
>> https://docs.google.com/spreadsheets/d/1qk-N5vjXvbcEk68GjbkSZTR8AGqyNUM-oLFo_ZXBpJw/edit#gid=2053422529
>>
>


Re: Precomit broken due to style violation. Why are failures getting past precommit?

2019-01-29 Thread Michael Luckey
As currently we miss prominent exposures of check style warnings, I added 
another post build action to publish check style and findbugs warnings. [1]

This will lead to
- show check style/findbugs status right on the status page


- links to followup pages with some graphs

This could help to not hide those warnings. Wdyt?

Of course, if we find this useful, we might extend to other precommits and/or 
other tools. E.g. findbugs might be useless anyway, as error prone will fail 
compile anyway?

[1] https://github.com/apache/beam/pull/7666

> On 24. Jan 2019, at 17:56, Scott Wegner  wrote:
> 
> Yes, you're correct that PR#7571 [1] had a checkstyle violation when merged. 
> I didn't notice the checkstyle failure and I hit the merge button. Sorry 
> about that.
> 
> Here's where I went wrong:
> * The precommits showed one failing: Java. GitHub shows the status as a green 
> check or red X on the head commit for a PR [2].
> * Opening the Jenkins link [3], it shows "Test Result (1 failure/ +-0)", and 
> clicking on that shows the failing test was testMatchWatchForNewFiles [4]
> * I recognized this as BEAM-6491 [5] and decided not to block the PR since it 
> should be unrelated. So I hit merge.
> 
> What I didn't do was click on the Gradle Build Scan link [6], which shows 
> that :beam-runners-direct-java:checkstyleMain failed as well.
> 
> I take the blame for letting this in, and I'll follow-up to make sure it gets 
> fixed. I also think that this is an easy mistake to make. Some ideas to 
> decrease the chances:
> 
> a) Split out checkstyle and other static analysis as a separate pre-commit 
> from running tests. Because Jenkins reports the test report separately and 
> more prominently, it's easy to miss other failures. We already split out 
> Spotless and Rat, which also provides the value of giving a faster signal on 
> those checks.
> 
> b) Have a stronger policy about not merging when tests are red. I just 
> checked and this is actually the policy already [7], but exceptions are 
> regularly made for flaky or unrelated test failures (evidence: each red X on 
> the master commit history [8]). Right now it's up to a human to make the call 
> on, and us humans are prone to make mistakes. We could consider enforcing the 
> policy and configure GitHub to require all checks passing before merge. This 
> will make flaky tests more painful, though it will also provide a stronger 
> incentive to fix the flaky tests.
> 
> [1] https://github.com/apache/beam/pull/7571 
> 
> [2] https://github.com/apache/beam/pull/7571/commits 
> 
> [3] https://builds.apache.org/job/beam_PreCommit_Java_Phrase/577/ 
> 
> [4] https://builds.apache.org/job/beam_PreCommit_Java_Phrase/577/testReport/ 
> 
> [5] https://jira.apache.org/jira/browse/BEAM-6491 
> 
> [6] https://scans.gradle.com/s/s3wdusaicauee 
> 
> [7] https://beam.apache.org/contribute/precommit-policies/#pull-requests 
> 
> [8] https://github.com/apache/beam/commits/master 
> 
> 
> On Wed, Jan 23, 2019 at 5:39 PM Alex Amato  > wrote:
> See: https://issues.apache.org/jira/browse/BEAM-6500 
> 
> 
> I think that this PR introduced the issue. Though I am not sure how to read 
> the test status. It looks like its marked with an X for the postcommit 
> status, but presumably the precommit was okay even though java precommit 
> appears to be broken now? Is there any explanation as to how this PR got 
> through?
> https://github.com/apache/beam/pull/7571 
> 
> 
> Today there have been numerous issues with presubmit. I would just like to 
> understand if there is some underlying issue going on here missing some 
> checks when we merge. Any ideas why this keeps occurring?
> 
> Thanks
> Alex
> 
> 
> -- 
> 
> 
> 
> 
> Got feedback? tinyurl.com/swegner-feedback 
> 



Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

2019-01-29 Thread Kenneth Knowles
On Mon, Jan 28, 2019 at 5:25 AM Łukasz Gajowy  wrote:

> IMHO, I don't think committers spend time watching new PRs coming up, but
> they more likely act when pinged. So, we may need some automation in case a
> contributor do not use github reviewed proposal. Auto reviewer assignment
> seem too much but modifying the PR template to add a sentence such as
> "please pickup a reviewer in the proposed list" could be enough.
> WDYT ?
>
> and
>
> (1) A sentence in the PR template suggesting adding a reviewer. (easy)
>
>
> +100! Let's improve the message in the PR template. It costs nothing and
> can help a lot. I'd say it should be in bold letters as this is super
> important.
>

There is already a message. Is it unclear? Can you rephrase it to something
better?

Kenn



> Maybe this is also worth reconsidering if auto reviewer assignment (or at
> least some form of it) is a bad idea. We can assign committers (the most
> "hardcore" option, maybe too much) or ping them in emails/github comments
> if there's inactivity in pull requests (the soft one but requires a bot to
> be implemented). The way I see this is that such auto assigned reviewer
> could always say "I have lots on my plate" but suggest someone else to take
> care of the PR. This way the problem that nobody is mentioned by the PR
> author is completely gone. Other than that, such an approach feels
> efficient to me because it's more "in person" (similar to what Robert
> said).
>
> It's certainly disheartening as a
> reviewer to put time into reviewing a PR and then the author doesn't
> bother to even respond, or (as has happened to me) be told "hey, this
> wasn't ready for review yet."
>
> As for "this wasn't ready for review" - there are sometimes situations
> that require a PR to be opened before they are actually completed
> (especially when working with Jenkins jobs). Given that there might be
> misunderstandings authors of such commits should give a clear message
> saying "do not merge yet" or "not ready for review" in title or comments or
> even close such PR and reopen until the change is ready. It's all about
> giving a clear signal to others.
>
> Maybe we should mention it in guidelines/PR message too to avoid
> situations like this?
>
> Łukasz
>
>
> pon., 28 sty 2019 o 11:30 Robert Bradshaw 
> napisał(a):
>
>> On Mon, Jan 28, 2019 at 10:37 AM Etienne Chauchot 
>> wrote:
>> >
>> > Sure it's a pity than this PR got unnoticed and I think it is a
>> combination of factors (PR date around Christmas, the fact that the author
>> forgot - AFAIK - to ping a reviewer in either the PR or the ML).
>> >
>> > I agree with Rui's proposal to enhance visibility of the "how to get a
>> reviewed" process.
>> >
>> > IMHO, I don't think committers spend time watching new PRs coming up,
>> but they more likely act when pinged. So, we may need some automation in
>> case a contributor do not use github reviewed proposal. Auto reviewer
>> assignment seem too much but modifying the PR template to add a sentence
>> such as "please pickup a reviewer in the proposed list" could be enough.
>> > WDYT ?
>>
>> +1
>>
>> I see two somewhat separable areas of improvement:
>>
>> (1) Getting a reviewer assigned to a PR, and
>> (2) Expectations of feedback/timeliness from a reviewer once it has
>> been assigned.
>>
>> The first is the motivation for this thread, but I think we're
>> suffering on the second as well.
>>
>> Given the reactions here, it sounds like most of us are just as
>> unhappy this happened as the author, and would be happy to pitch in
>> and improve things.
>>
>> I agree with Kenn that just adding to more the contributor guide
>> always help because a contributor guide with everything one might need
>> to know is the least likely to actually be read in its entirety.
>> Rather it's useful to provide such guidance at the point that it's
>> needed. Specifically, I would like to see
>>
>> (1) A sentence in the PR template suggesting adding a reviewer. (easy)
>> (2) An automated recommendation for suggesting good candidate
>> reviewers (if we deem Github's suggestions insufficient). (harder)
>> (3) A bot that follows up on PRs after 1 week(?) noting the lack of
>> action and suggesting (and, implicitly but importantly
>> permission/expectation) that the author bring the PR to the list.
>> (medium)
>>
>> We could also have automated emails like the Beam Dependency Check
>> Report, but automated emails are much easier to ignore than personal
>> ones. Having the author ping dev@ has the added advantage that it
>> gives the author something they can do to move the PR forward, and
>> provides a clear signal that this is a PR someone cares enough about
>> to prioritize it above others. (It's certainly disheartening as a
>> reviewer to put time into reviewing a PR and then the author doesn't
>> bother to even respond, or (as has happened to me) be told "hey, this
>> wasn't ready for review yet." On the other hand it's rewarding to help
>> someone, especially a first time 

Re: GSOC - Summer of Code, on Beam?

2019-01-29 Thread Kenneth Knowles
I believe for the ASF process the target is January 31, no? So basically if
you have an idea, file it now!

Kenn

On Wed, Jan 16, 2019 at 2:23 AM Ismaël Mejía  wrote:

> Little reminder for the interested parties:
> GSoC 2019 Org applications are now open!
> Deadline is at February 6 at 20:00 UTC
> For mentors, remember that apart of the standard process you must
> apply via filling the Apache spreadsheet.
>
> On Fri, Dec 14, 2018 at 6:44 PM Kenneth Knowles  wrote:
> >
> > I put together a (currently empty) saved search for Beam's GSoC issues:
> https://issues.apache.org/jira/issues/?filter=12345337
> >
> > Kenn
> >
> > On Wed, Dec 12, 2018 at 1:10 AM Ismaël Mejía  wrote:
> >>
> >> Oh I had not seen that the announce was official, so time to get ready.
> >>
> https://opensource.googleblog.com/2018/11/google-summer-of-code-15-years-strong.html
> >>
> >> Mentors should have proposals ready around January 15, 2019. Remember
> >> timeline matters.
> >> https://developers.google.com/open-source/gsoc/timeline
> >>
> >> On Tue, Dec 11, 2018 at 6:14 PM Ismaël Mejía  wrote:
> >> >
> >> > You have to register the concrete proposal, no need to register the
> >> > project since the organization (ASF) is already part of GSoC.
> >> > On Tue, Dec 11, 2018 at 12:42 PM Maximilian Michels 
> wrote:
> >> > >
> >> > > I think that's a great idea if we can find good candidates. Do we
> have to
> >> > > register the project to be able to receive applications from
> students?
> >> > >
> >> > > On 07.12.18 16:44, Ismaël Mejía wrote:
> >> > > > Last year we had two proposals. Kenneth proposal around SQL was
> >> > > > accepted and if I remember correctly a success.
> >> > > >
> >> > > > For students interested you should do the complete process via the
> >> > > > GSoC website and look for the ‘gsoc’ label.
> >> > > >
> >> > > > For committers who want to mentor students you have to subscribe
> into
> >> > > > the GSoC website, but also to the mentors@ mailing list, where
> you
> >> > > > should fill the proposal through a document shared there (and yes
> this
> >> > > > is not documented at all, need to fix that). This document is
> prepared
> >> > > > by the ASF because is the full organization (not the project) who
> >> > > > requests the participation. Sadly I was not aware of this process
> and
> >> > > > I missed the Apache deadline (some days before the GSoC deadline)
> last
> >> > > > year and for that reason my proposal did not get accepted. So pay
> >> > > > attention to that Pablo (or the others that want to mentor some
> >> > > > students).
> >> > > >
> >> > > > On Thu, Dec 6, 2018 at 8:21 PM jhzgg2...@gmail.com <
> jhzgg2...@gmail.com> wrote:
> >> > > >>
> >> > > >>
> >> > > >>
> >> > > >> On 2018/12/05 00:29:48, Pablo Estrada 
> wrote:
> >> > > >>> Hi Austin!
> >> > > >>> Thanks a lot for surfacing this. I participated in GSOC as a
> student a
> >> > > >>> couple times, and loved it. This being my first time around as
> a committer,
> >> > > >>> I'm excited to try and help.
> >> > > >>>
> >> > > >>> I think, for starters, it may be good to find issues in JIRA to
> label with
> >> > > >>> "gsoc", so please everyone who knows of good candidate project
> issues,
> >> > > >>> label them with "gsoc".
> >> > > >>>
> >> > > >>> And then we can find mentors for these issues, and start
> helping students
> >> > > >>> in the application process.
> >> > > >>>
> >> > > >>> Best
> >> > > >>> -P.
> >> > > >>>
> >> > > >>> On Tue, Dec 4, 2018 at 3:46 PM Austin Bennett <
> whatwouldausti...@gmail.com>
> >> > > >>> wrote:
> >> > > >>>
> >> > >  Would it make sense to have any GSOC students for next summer
> work on
> >> > >  Beam?  Do we have some candidate things that would be suitable
> and
> >> > >  sufficiently discrete projects?
> >> > > 
> >> > >  Initial applications for organizations not even open for about
> a month,
> >> > >  though thought worth getting a sense from the group.
> >> > > 
> >> > >  A bit of info:
> >> > >  https://summerofcode.withgoogle.com/archive/
> >> > > 
> >> > > 
> https://opensource.googleblog.com/2018/11/google-summer-of-code-15-years-strong.html
> >> > > 
> >> > > 
> >> > > 
> >> > > 
> >> > > >>> Hi Pablo!
> >> > > >>> I am a junior majoring in CS and interested in Apache Beam and
> data process. I hope to >participate in GSOC and work on Beam next summer.
> Could you give me some advice on
> >> > > >>> how to prepare for it? Thanks a lot.
> >> > > >>
>


Re: Enforce javadoc comments in public methods?

2019-01-29 Thread Kenneth Knowles
It sounds like there's widespread practices in our codebase that this new
restriction might improve.

Instead of /** Constructs XX object */ you could add a @deprecated and/or a
TODO to migrate the code.

Kenn

On Mon, Jan 28, 2019 at 11:46 PM Reuven Lax  wrote:

> The problem is existing code. I find that if I touch existing code that
> has a constructor, I am then forced to add a Javadoc on the constructor.
> Usually this Javadoc is "Constructs XX object."
>
> On Mon, Jan 28, 2019 at 5:07 PM Kenneth Knowles  wrote:
>
>> Half agree, half disagree
>>
>> Disagree: best practice is to make your constructors private and have
>> distinct static methods for various modes of instantiation, which will then
>> require Javadoc.
>>
>> Agree: once they are private no javadoc is needed :-) and there's often
>> only one "most general" constructor that all the static methods use in
>> different ways
>>
>> Kenn
>>
>> On Mon, Jan 28, 2019 at 5:03 PM Ruoyun Huang  wrote:
>>
>>> Fair point. Looking at JavaDocMethod spec [1], unfortunately there is no
>>> properties available for this tweak.
>>>
>>> Let me dig a bit more to see whether this can be done via suppression.
>>>
>>> [1] http://checkstyle.sourceforge.net/config_javadoc.html#JavadocMethod
>>>
>>> On Mon, Jan 28, 2019 at 4:36 PM Reuven Lax  wrote:
>>>
 This appears to be forcing us to set javadoc on constructors as well,
 which is usually pointless. Can we exclude constructor methods from this
 check?

 On Wed, Jan 23, 2019 at 5:40 PM Ruoyun Huang  wrote:

> Our recent change is on "JavaDocMethod", which not turned on yet. Not
> relevant to this error here.
>
> The one throws error is "javaDocType". it has been there for a while
> ,
> which is for public class javadoc missing.  Yeah, I am curious as well why
> preCommit didn't catch this one.
>
>
>
> On Wed, Jan 23, 2019 at 5:28 PM Alex Amato  wrote:
>
>> Did their happen to be a short time window where some missing Javadoc
>> comments went in? I am now seeing precommit fail due to code I didn't
>> modify.
>>
>>
>> https://scans.gradle.com/s/nwgb7xegklwqo/console-log?task=:beam-runners-direct-java:checkstyleMain
>>
>>
>>
>> On Wed, Jan 23, 2019 at 2:34 PM Ruoyun Huang 
>> wrote:
>>
>>> Trying to understand your suggestion. By saying "break that
>>> dependency", do you mean moving checkstyle out of Java PreCommit?
>>>
>>> currently we do have checkstyle as part of  ":check".  It seems to
>>> me "check" does minimal amount of essential works (correct me If I am
>>> wrong), much less than what PreCommit does.
>>>
>>> On Wed, Jan 23, 2019 at 12:20 PM Kenneth Knowles 
>>> wrote:
>>>
 It is always a bummer when the Java PreCommit fails due to style
 checking. Can we get this to run separately and quicker? I notice it
 depends on compileJava. I cannot remember why that is, but I recall it 
 is a
 legitimate reason. Nonetheless, can we break that dependency somehow?

 Kenn

 On Wed, Jan 16, 2019 at 6:42 PM Ruoyun Huang 
 wrote:

> Hi, everyone,
>
>
> To make sure we move forward to a clean state where we catch
> violations in any new PR, we created this change:
> https://github.com/apache/beam/pull/7532
>
> This PR makes checkstyle to report error on missing javadocs. For
> existing violations, we explicitly added them as suppression rules, 
> down to
> which line in the code.
>
> The caveat is, once this PR is merged, whoever make update to any
> file in the list, will very likely have to fix the existing violation 
> for
> that file.  :-) Hope this sounds like a reasonable idea to move 
> forward.
>
> In the meanwhile, I will try to address the items in the list (if
> I can). And over time, I will get back to this and remove those
> suppressions no longer needed (created JIRA-6446 for tracking
> purpose), until all of them are fixed.
>
> On Wed, Jan 9, 2019 at 10:57 PM Ruoyun Huang 
> wrote:
>
>> created a PR: https://github.com/apache/beam/pull/7454
>>
>> Note instead of having separated checkstyle specs for Main versus
>> Test, this PR simply uses suppression to turn off JavaDocComment for 
>> test
>> files.
>>
>> If this PR draft looks good, then next step I will commit another
>> change that:
>> 1) throw error on violations (now just warning to keep PR green).
>> 2) List all the violations explicitly in a suppression list, and
>> let area contributors/owners address 

Re: ContainerLaunchException in precommit [BEAM-6497]

2019-01-29 Thread Kenneth Knowles
I retract my statement. I failed at web browsing.

On Tue, Jan 29, 2019 at 3:14 PM Kenneth Knowles  wrote:

> Version 18.10.3 no longer appears on the linked page.
>
> On Tue, Jan 29, 2019 at 3:08 PM David Rieber  wrote:
>
>> I am consistently hitting that error on this PR:
>> https://github.com/apache/beam/pull/7631
>>
>>
>> On Thu, Jan 24, 2019 at 9:14 AM Alex Amato  wrote:
>>
>>> I have just seen it randomly occur on presubmits. So I don't have a
>>> reliable repro, unfortunately.
>>> It may be a specific environmental issue to the beam1 machine the tests
>>> ran on?
>>> https://builds.apache.org/job/beam_PreCommit_Java_Commit/3722/
>>>
>>>
>>> On Thu, Jan 24, 2019 at 8:16 AM Gleb Kanterov  wrote:
>>>
 I'm wondering if anybody can reproduce this issue. The build has failed
 once because testcontainers didn't pull docker image. If we use caching
 proxy for docker, it could be a reason for that. I didn't find any similar
 known issue in testcontainers fixed recently, but just in case, I bumped
 testcontainers to use never docker-java.

 https://github.com/apache/beam/pull/7610

 On Thu, Jan 24, 2019 at 12:27 AM Alex Amato  wrote:

> Thank you Gleb, appreciate it.
>
> On Wed, Jan 23, 2019 at 2:40 PM Gleb Kanterov 
> wrote:
>
>> I'm looking into it. This image exists in docker hub [1], but for
>> some reason, it wasn't picked up.
>>
>> [1] https://hub.docker.com/r/yandex/clickhouse-server/tags
>>
>> On Wed, Jan 23, 2019 at 10:01 PM Alex Amato 
>> wrote:
>>
>>>
>>>1.
>>>   See: BEAM-6497 
>>>   1. This is also causing issues blocking precommits.
>>>   2.
>>>   Seems to be caused by this failure to locate the image. Are
>>>  these stored somewhere or built by the build process? Any idea 
>>> why these
>>>  are failing?
>>>
>>>  Caused by: 
>>> com.github.dockerjava.api.exception.NotFoundException: {"message":"No 
>>> such image: yandex/clickhouse-server:18.10.3"}
>>>
>>>
>>>
>>>
>>
>> --
>> Cheers,
>> Gleb
>>
>

 --
 Cheers,
 Gleb

>>>


Re: ContainerLaunchException in precommit [BEAM-6497]

2019-01-29 Thread Kenneth Knowles
Version 18.10.3 no longer appears on the linked page.

On Tue, Jan 29, 2019 at 3:08 PM David Rieber  wrote:

> I am consistently hitting that error on this PR:
> https://github.com/apache/beam/pull/7631
>
>
> On Thu, Jan 24, 2019 at 9:14 AM Alex Amato  wrote:
>
>> I have just seen it randomly occur on presubmits. So I don't have a
>> reliable repro, unfortunately.
>> It may be a specific environmental issue to the beam1 machine the tests
>> ran on?
>> https://builds.apache.org/job/beam_PreCommit_Java_Commit/3722/
>>
>>
>> On Thu, Jan 24, 2019 at 8:16 AM Gleb Kanterov  wrote:
>>
>>> I'm wondering if anybody can reproduce this issue. The build has failed
>>> once because testcontainers didn't pull docker image. If we use caching
>>> proxy for docker, it could be a reason for that. I didn't find any similar
>>> known issue in testcontainers fixed recently, but just in case, I bumped
>>> testcontainers to use never docker-java.
>>>
>>> https://github.com/apache/beam/pull/7610
>>>
>>> On Thu, Jan 24, 2019 at 12:27 AM Alex Amato  wrote:
>>>
 Thank you Gleb, appreciate it.

 On Wed, Jan 23, 2019 at 2:40 PM Gleb Kanterov  wrote:

> I'm looking into it. This image exists in docker hub [1], but for some
> reason, it wasn't picked up.
>
> [1] https://hub.docker.com/r/yandex/clickhouse-server/tags
>
> On Wed, Jan 23, 2019 at 10:01 PM Alex Amato 
> wrote:
>
>>
>>1.
>>   See: BEAM-6497 
>>   1. This is also causing issues blocking precommits.
>>   2.
>>   Seems to be caused by this failure to locate the image. Are
>>  these stored somewhere or built by the build process? Any idea 
>> why these
>>  are failing?
>>
>>  Caused by: 
>> com.github.dockerjava.api.exception.NotFoundException: {"message":"No 
>> such image: yandex/clickhouse-server:18.10.3"}
>>
>>
>>
>>
>
> --
> Cheers,
> Gleb
>

>>>
>>> --
>>> Cheers,
>>> Gleb
>>>
>>


Re: ContainerLaunchException in precommit [BEAM-6497]

2019-01-29 Thread David Rieber
I am consistently hitting that error on this PR:
https://github.com/apache/beam/pull/7631


On Thu, Jan 24, 2019 at 9:14 AM Alex Amato  wrote:

> I have just seen it randomly occur on presubmits. So I don't have a
> reliable repro, unfortunately.
> It may be a specific environmental issue to the beam1 machine the tests
> ran on?
> https://builds.apache.org/job/beam_PreCommit_Java_Commit/3722/
>
>
> On Thu, Jan 24, 2019 at 8:16 AM Gleb Kanterov  wrote:
>
>> I'm wondering if anybody can reproduce this issue. The build has failed
>> once because testcontainers didn't pull docker image. If we use caching
>> proxy for docker, it could be a reason for that. I didn't find any similar
>> known issue in testcontainers fixed recently, but just in case, I bumped
>> testcontainers to use never docker-java.
>>
>> https://github.com/apache/beam/pull/7610
>>
>> On Thu, Jan 24, 2019 at 12:27 AM Alex Amato  wrote:
>>
>>> Thank you Gleb, appreciate it.
>>>
>>> On Wed, Jan 23, 2019 at 2:40 PM Gleb Kanterov  wrote:
>>>
 I'm looking into it. This image exists in docker hub [1], but for some
 reason, it wasn't picked up.

 [1] https://hub.docker.com/r/yandex/clickhouse-server/tags

 On Wed, Jan 23, 2019 at 10:01 PM Alex Amato  wrote:

>
>1.
>   See: BEAM-6497 
>   1. This is also causing issues blocking precommits.
>   2.
>   Seems to be caused by this failure to locate the image. Are
>  these stored somewhere or built by the build process? Any idea 
> why these
>  are failing?
>
>  Caused by: 
> com.github.dockerjava.api.exception.NotFoundException: {"message":"No 
> such image: yandex/clickhouse-server:18.10.3"}
>
>
>
>

 --
 Cheers,
 Gleb

>>>
>>
>> --
>> Cheers,
>> Gleb
>>
>


Re: [VOTE] Release 2.10.0, release candidate #1

2019-01-29 Thread Kenneth Knowles
Yes, the instructions for building the wheels includes inputting my ASF
credentials into Travis-CI. I've been trying to understand why and what I
can do instead.

(The release guide says that the release script builds the binaries, but
from what I can tell it does not. This makes sense because the instructions
are highly manual too.)

Kenn

On Tue, Jan 29, 2019 at 12:38 AM Robert Bradshaw 
wrote:

> The artifacts and signatures look good. But we're missing Python wheels.
>
>
> On Tue, Jan 29, 2019 at 6:08 AM Kenneth Knowles  wrote:
> >
> > Ah, I did not close the staging repository. Thanks for letting me know.
> Try now.
> >
> > Kenn
> >
> > On Mon, Jan 28, 2019 at 2:31 PM Ismaël Mejía  wrote:
> >>
> >> I think there is an issue, [4] does not open?
> >>
> >> On Mon, Jan 28, 2019 at 6:24 PM Kenneth Knowles 
> wrote:
> >> >
> >> > Hi everyone,
> >> >
> >> > Please review and vote on the release candidate #1 for the version
> 2.10.0, as follows:
> >> >
> >> > [ ] +1, Approve the release
> >> > [ ] -1, Do not approve the release (please provide specific comments)
> >> >
> >> > The complete staging area is available for your review, which
> includes:
> >> > * JIRA release notes [1],
> >> > * the official Apache source release to be deployed to
> dist.apache.org [2], which is signed with the key with fingerprint
> 6ED551A8AE02461C [3],
> >> > * all artifacts to be deployed to the Maven Central Repository [4],
> >> > * source code tag "v2.10.0-RC1" [5],
> >> > * website pull request listing the release [6] and publishing the API
> reference manual [7].
> >> > * Python artifacts are deployed along with the source release to the
> dist.apache.org [2].
> >> > * Validation sheet with a tab for 2.10.0 release to help with
> validation [7].
> >> >
> >> > The vote will be open for at least 72 hours. It is adopted by
> majority approval, with at least 3 PMC affirmative votes.
> >> >
> >> > Thanks,
> >> > Kenn
> >> >
> >> > [1]
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12319527=12344540
> >> > [2] https://dist.apache.org/repos/dist/dev/beam/2.10.0/
> >> > [3] https://dist.apache.org/repos/dist/release/beam/KEYS
> >> > [4]
> https://repository.apache.org/content/repositories/orgapachebeam-1056/
> >> > [5] https://github.com/apache/beam/tree/v2.10.0-RC1
> >> > [6] https://github.com/apache/beam/pull/7651/files
> >> > [7] https://github.com/apache/beam-site/pull/585
> >> > [8]
> https://docs.google.com/spreadsheets/d/1qk-N5vjXvbcEk68GjbkSZTR8AGqyNUM-oLFo_ZXBpJw/edit#gid=2053422529
>


Fwd: Logging with Key-Value Tags

2019-01-29 Thread Akshay Balwally
Hi,
I have logging statements in my Beam python code (using Flink). To improve
debugging, I add tags to the log statement, which are then indexable on my
log tool (kibana). However, the log statements that come out are not
properly formatted.

For example, statements like:

logger.exception('this is the desired msg field', kv={'some_tag': x,
'some_other_tag': y})

Comes out with a log message like:

json={"some_tag": x, "some_other_tag": y} msg=this is the desired msg field
I want the message to be simply:

this is the desired msg field

Where the two tags are separate, indexable attributes used by the log tool
(kibana).


More concretely, the below screenshot shows output from logging outside the
beam pipeline- i.e. the desired behavior. Note that the "
pricing.conversion_method" and "pricing.pricing_region" fields are tags
that I added in code.
[image: image.png]
 However, logging inside the Beam pipeline creates entries like this:
[image: image.png]
Where I want "pricingrealtime_region" and "model_version" to be separate.


In general, it seems like the kv dictionary, which I'm passing to the
logging client, is not being properly parsed, and the fields to be indexed
are not being removed. I'm ~guessing~ that this
https://github.com/apache/beam/blob/master/model/fn-execution/src/main/proto/beam_fn_api.proto#L987
needs to be a list of key-value pairs (rather than just a string). Does
anyone have any idea or plans to fix this?

Thanks,
- Akshay


Re: [ANNOUNCE] New PMC member: Etienne Chauchot

2019-01-29 Thread Chamikara Jayalath
Congrats Etienne!!

On Tue, Jan 29, 2019 at 6:30 AM Etienne Chauchot 
wrote:

> Thanks for the warm welcome guys !
>
> Etienne
>
> Le lundi 28 janvier 2019 à 14:28 +0100, Łukasz Gajowy a écrit :
>
> Thanks for your great work and congratulations! :)
>
> pon., 28 sty 2019 o 12:01 Gleb Kanterov  napisał(a):
>
> Congratulations Etienne!
>
>
> On Mon, Jan 28, 2019 at 11:36 AM Maximilian Michels 
> wrote:
>
> Congrats Etienne! It's been great to work with you.
>
> On 26.01.19 07:16, Ismaël Mejía wrote:
> > Congratulations Etienne!
> >
> > Le sam. 26 janv. 2019 à 06:42, Reuven Lax  > > a écrit :
> >
> > Welcome!
> >
> > On Fri, Jan 25, 2019 at 9:30 PM Pablo Estrada  > > wrote:
> >
> > Congrats Etienne :)
> >
> > On Fri, Jan 25, 2019, 9:24 PM Trần Thành Đạt <
> dattran.v...@gmail.com
> >  wrote:
> >
> > Congratulations Etienne!
> >
> > On Sat, Jan 26, 2019 at 12:08 PM Thomas Weise <
> t...@apache.org
> > > wrote:
> >
> > Congrats, félicitations!
> >
> >
> > On Fri, Jan 25, 2019 at 3:06 PM Scott Wegner <
> sc...@apache.org
> > > wrote:
> >
> > Congrats Etienne!
> >
> > On Fri, Jan 25, 2019 at 2:34 PM Tim
> >  > > wrote:
> >
> > Congratulations Etienne!
> >
> > Tim
> >
> >  > On 25 Jan 2019, at 23:00, Kenneth Knowles
> > mailto:k...@apache.org>>
> wrote:
> >  >
> >  > Hi all,
> >  >
> >  > Please join me and the rest of the Beam PMC in
> > welcoming Etienne Chauchot to join the PMC.
> >  >
> >  > Etienne introduced himself to dev@ in
> September of
> > 2017 and over the years has contributed to Beam
> in many
> > ways - connectors, performance, design
> discussion,
> > talks, code reviews, and I'm sure I cannot list
> them
> > all. He already has a major impact on the
> direction of Beam.
> >  >
> >  > Thanks for being a part of Beam, Etienne!
> >  >
> >  > Kenn
> >
> >
> >
> > --
> >
> >
> >
> >
> > Got feedback? tinyurl.com/swegner-feedback
> > 
> >
>
>
>
>


Re: [ANNOUNCE] New PMC member: Etienne Chauchot

2019-01-29 Thread Etienne Chauchot
Thanks for the warm welcome guys !
Etienne
Le lundi 28 janvier 2019 à 14:28 +0100, Łukasz Gajowy a écrit :
> Thanks for your great work and congratulations! :)
> 
> pon., 28 sty 2019 o 12:01 Gleb Kanterov  napisał(a):
> > Congratulations Etienne! 
> > 
> > 
> > 
> > On Mon, Jan 28, 2019 at 11:36 AM Maximilian Michels  wrote:
> > > Congrats Etienne! It's been great to work with you.
> > > 
> > > 
> > > 
> > > On 26.01.19 07:16, Ismaël Mejía wrote:
> > > 
> > > > Congratulations Etienne!
> > > 
> > > > 
> > > 
> > > > Le sam. 26 janv. 2019 à 06:42, Reuven Lax  > > 
> > > > > a écrit :
> > > 
> > > > 
> > > 
> > > > Welcome!
> > > 
> > > > 
> > > 
> > > > On Fri, Jan 25, 2019 at 9:30 PM Pablo Estrada  > > 
> > > > > wrote:
> > > 
> > > > 
> > > 
> > > > Congrats Etienne :)
> > > 
> > > > 
> > > 
> > > > On Fri, Jan 25, 2019, 9:24 PM Trần Thành Đạt 
> > > >  > > 
> > > >  wrote:
> > > 
> > > > 
> > > 
> > > > Congratulations Etienne!
> > > 
> > > > 
> > > 
> > > > On Sat, Jan 26, 2019 at 12:08 PM Thomas Weise 
> > > >  > > 
> > > > > wrote:
> > > 
> > > > 
> > > 
> > > > Congrats, félicitations!
> > > 
> > > > 
> > > 
> > > > 
> > > 
> > > > On Fri, Jan 25, 2019 at 3:06 PM Scott Wegner 
> > > >  > > 
> > > > > wrote:
> > > 
> > > > 
> > > 
> > > > Congrats Etienne!
> > > 
> > > > 
> > > 
> > > > On Fri, Jan 25, 2019 at 2:34 PM Tim
> > > 
> > > >  > > 
> > > > > wrote:
> > > 
> > > > 
> > > 
> > > > Congratulations Etienne!
> > > 
> > > > 
> > > 
> > > > Tim
> > > 
> > > > 
> > > 
> > > >  > On 25 Jan 2019, at 23:00, Kenneth Knowles
> > > 
> > > > mailto:k...@apache.org>> 
> > > > wrote:
> > > 
> > > >  >
> > > 
> > > >  > Hi all,
> > > 
> > > >  >
> > > 
> > > >  > Please join me and the rest of the Beam PMC 
> > > > in
> > > 
> > > > welcoming Etienne Chauchot to join the PMC.
> > > 
> > > >  >
> > > 
> > > >  > Etienne introduced himself to dev@ in 
> > > > September of
> > > 
> > > > 2017 and over the years has contributed to Beam 
> > > > in many
> > > 
> > > > ways - connectors, performance, design 
> > > > discussion,
> > > 
> > > > talks, code reviews, and I'm sure I cannot list 
> > > > them
> > > 
> > > > all. He already has a major impact on the 
> > > > direction of Beam.
> > > 
> > > >  >
> > > 
> > > >  > Thanks for being a part of Beam, Etienne!
> > > 
> > > >  >
> > > 
> > > >  > Kenn
> > > 
> > > > 
> > > 
> > > > 
> > > 
> > > > 
> > > 
> > > > -- 
> > > 
> > > > 
> > > 
> > > > 
> > > 
> > > > 
> > > 
> > > > 
> > > 
> > > > Got feedback? tinyurl.com/swegner-feedback
> > > 
> > > > 
> > > 
> > > > 
> > > 
> > 
> > 


Re: [VOTE] Release 2.10.0, release candidate #1

2019-01-29 Thread Robert Bradshaw
The artifacts and signatures look good. But we're missing Python wheels.


On Tue, Jan 29, 2019 at 6:08 AM Kenneth Knowles  wrote:
>
> Ah, I did not close the staging repository. Thanks for letting me know. Try 
> now.
>
> Kenn
>
> On Mon, Jan 28, 2019 at 2:31 PM Ismaël Mejía  wrote:
>>
>> I think there is an issue, [4] does not open?
>>
>> On Mon, Jan 28, 2019 at 6:24 PM Kenneth Knowles  wrote:
>> >
>> > Hi everyone,
>> >
>> > Please review and vote on the release candidate #1 for the version 2.10.0, 
>> > as follows:
>> >
>> > [ ] +1, Approve the release
>> > [ ] -1, Do not approve the release (please provide specific comments)
>> >
>> > The complete staging area is available for your review, which includes:
>> > * JIRA release notes [1],
>> > * the official Apache source release to be deployed to dist.apache.org 
>> > [2], which is signed with the key with fingerprint 6ED551A8AE02461C [3],
>> > * all artifacts to be deployed to the Maven Central Repository [4],
>> > * source code tag "v2.10.0-RC1" [5],
>> > * website pull request listing the release [6] and publishing the API 
>> > reference manual [7].
>> > * Python artifacts are deployed along with the source release to the 
>> > dist.apache.org [2].
>> > * Validation sheet with a tab for 2.10.0 release to help with validation 
>> > [7].
>> >
>> > The vote will be open for at least 72 hours. It is adopted by majority 
>> > approval, with at least 3 PMC affirmative votes.
>> >
>> > Thanks,
>> > Kenn
>> >
>> > [1] 
>> > https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12319527=12344540
>> > [2] https://dist.apache.org/repos/dist/dev/beam/2.10.0/
>> > [3] https://dist.apache.org/repos/dist/release/beam/KEYS
>> > [4] https://repository.apache.org/content/repositories/orgapachebeam-1056/
>> > [5] https://github.com/apache/beam/tree/v2.10.0-RC1
>> > [6] https://github.com/apache/beam/pull/7651/files
>> > [7] https://github.com/apache/beam-site/pull/585
>> > [8] 
>> > https://docs.google.com/spreadsheets/d/1qk-N5vjXvbcEk68GjbkSZTR8AGqyNUM-oLFo_ZXBpJw/edit#gid=2053422529