Re: script, GPL, container question

[Gian Merlino]

I did some looking around and found this:
https://issues.apache.org/jira/browse/INFRA-12781. It seems that the Apache
Infra folks will set up DockerHub configs for projects like us, in such a
way that they build automatically off release tags. So it does seem pretty
easy; it looks like once we have a working Dockerfile we just need to raise
an INFRA ticket to get them to set that up.

> how do i drive to consensus on the mysql thing?

One way is try to encourage more people to chime in here (you are already
doing this by writing emails). Another is to reach out and ask the
Incubator PMC (email gene...@incubator.apache.org) or Apache Legal (raise a
LEGAL ticket in JIRA) for advice. I would probably go for the Incubator PMC
first, since the audience is a bit larger and this may have come up before.

On Fri, Jan 25, 2019 at 1:24 PM Don Bowman  wrote:

> On Fri, 25 Jan 2019 at 16:07, Gian Merlino  wrote:
>
> > For Q1 - I have the feeling that if we asked the powers that be at Apache
> > for an opinion on bundling the MySQL connector that they would not be
> fans
> > of the idea -- mostly because it is not allowed for binary tarball
> > releases, and I don't see why it would be different for Docker releases.
> So
> > because of that, I wouldn't be comfortable bundling the mysql-connector
> jar
> > unless we actually _did_ ask the powers that be, and they said it's ok.
> The
> > powers that be are probably either ASF Legal or the Incubator PMC.
> >
> > For Q2 - IMO precedent established by other projects means this is not
> > likely an issue. Probably because of the mere-aggregation reason you
> > brought up.
> >
> > If we want to release official Docker images then this will also need to
> be
> > incorporated into our build process. Don are you interested in
> researching
> > / proposing how that might be done? Anyone else - should we have a
> > discussion about whether we want official Docker images as part of our
> > release process? Personally, if it doesn't impose much additional burden
> on
> > release managers, and the image is something that is easy to run in
> > production in a way that we are comfortable supporting as a community, I
> am
> > for it. (I haven't reviewed the PR enough to have an opinion on that.)
> >
> >
> >
> Its pretty trivial to let the dockerhub run its own pipeline on any {tag |
> merge} in git.
> it does this automatically.
> Or, its not too hard to have travis have some keys to do a push and it in
> turn is gated
> by a {tag | merge}.
>
> some projects build the release on each 'tag' created.
> some build on tag matching pattern
> some build on any merge commit to master.
>
> IMO w/o a release of container to a public repo this is pointless, its what
> people
> expect.
>
> I don't have any particular domain knowledge in the area but am willing to
> do
> some work if it is identified as needing done.
>
> how do i drive to consensus on the mysql thing?
>

Re: script, GPL, container question

[Don Bowman]

On Fri, 25 Jan 2019 at 16:07, Gian Merlino  wrote:

> For Q1 - I have the feeling that if we asked the powers that be at Apache
> for an opinion on bundling the MySQL connector that they would not be fans
> of the idea -- mostly because it is not allowed for binary tarball
> releases, and I don't see why it would be different for Docker releases. So
> because of that, I wouldn't be comfortable bundling the mysql-connector jar
> unless we actually _did_ ask the powers that be, and they said it's ok. The
> powers that be are probably either ASF Legal or the Incubator PMC.
>
> For Q2 - IMO precedent established by other projects means this is not
> likely an issue. Probably because of the mere-aggregation reason you
> brought up.
>
> If we want to release official Docker images then this will also need to be
> incorporated into our build process. Don are you interested in researching
> / proposing how that might be done? Anyone else - should we have a
> discussion about whether we want official Docker images as part of our
> release process? Personally, if it doesn't impose much additional burden on
> release managers, and the image is something that is easy to run in
> production in a way that we are comfortable supporting as a community, I am
> for it. (I haven't reviewed the PR enough to have an opinion on that.)
>
>
>
Its pretty trivial to let the dockerhub run its own pipeline on any {tag |
merge} in git.
it does this automatically.
Or, its not too hard to have travis have some keys to do a push and it in
turn is gated
by a {tag | merge}.

some projects build the release on each 'tag' created.
some build on tag matching pattern
some build on any merge commit to master.

IMO w/o a release of container to a public repo this is pointless, its what
people
expect.

I don't have any particular domain knowledge in the area but am willing to
do
some work if it is identified as needing done.

how do i drive to consensus on the mysql thing?

Re: script, GPL, container question

[Gian Merlino]

For Q1 - I have the feeling that if we asked the powers that be at Apache
for an opinion on bundling the MySQL connector that they would not be fans
of the idea -- mostly because it is not allowed for binary tarball
releases, and I don't see why it would be different for Docker releases. So
because of that, I wouldn't be comfortable bundling the mysql-connector jar
unless we actually _did_ ask the powers that be, and they said it's ok. The
powers that be are probably either ASF Legal or the Incubator PMC.

For Q2 - IMO precedent established by other projects means this is not
likely an issue. Probably because of the mere-aggregation reason you
brought up.

If we want to release official Docker images then this will also need to be
incorporated into our build process. Don are you interested in researching
/ proposing how that might be done? Anyone else - should we have a
discussion about whether we want official Docker images as part of our
release process? Personally, if it doesn't impose much additional burden on
release managers, and the image is something that is easy to run in
production in a way that we are comfortable supporting as a community, I am
for it. (I haven't reviewed the PR enough to have an opinion on that.)

On Fri, Jan 25, 2019 at 12:53 PM Don Bowman  wrote:

> On Fri, 25 Jan 2019 at 13:17, Gian Merlino  wrote:
>
> > For Q1 the legal guidance as I understand it is that we can provide users
> > with instructions for how to get optional (L)GPL dependencies, but we
> can't
> > distribute them ourselves. Putting the mysql-connector in an Docker image
> > does feel like distribution…
> >
> > Q2 is an interesting question. I wonder if Apache has a policy on
> official
> > or semiofficial Docker containers that touches on the possibly thorny
> > licensing questions. It seems that they do exist for other projects,
> > though: https://hub.docker.com/u/apache. The Zeppelin one, for example,
> is
> > based on ubuntu so it must have plenty of GPL stuff in it:
> > https://hub.docker.com/r/apache/zeppelin/dockerfile. And it is presented
> > on
> > the Zeppelin page as an official thing:
> > https://zeppelin.apache.org/docs/0.7.0/install/docker.html.
> >
> > I dunno, it feels weird to me, and I am searching for evidence of these
> > issues having been explicitly discussed by other projects but have not
> > found it yet.
> >
> >
> >
> GPL does not attach by mere aggregation. [see GPL FAQ
> ]
> All linux is gpl, and all the containers are linux for all the other apache
> foundation things (maven, httpd, ...). even debian has bash in it, which is
> gpl.
>
> so I can either:
>
> a) continue as is. I want to get this on dockerhub auto-built, that's what
> he script does now. BTW, it downloads the gpl code from maven repository,
> which is also run by apache.
> b) remove it, support postgres only.
>
> both are ok w/ me I suppose.
>

Re: script, GPL, container question

[Don Bowman]

On Fri, 25 Jan 2019 at 13:17, Gian Merlino  wrote:

> For Q1 the legal guidance as I understand it is that we can provide users
> with instructions for how to get optional (L)GPL dependencies, but we can't
> distribute them ourselves. Putting the mysql-connector in an Docker image
> does feel like distribution…
>
> Q2 is an interesting question. I wonder if Apache has a policy on official
> or semiofficial Docker containers that touches on the possibly thorny
> licensing questions. It seems that they do exist for other projects,
> though: https://hub.docker.com/u/apache. The Zeppelin one, for example, is
> based on ubuntu so it must have plenty of GPL stuff in it:
> https://hub.docker.com/r/apache/zeppelin/dockerfile. And it is presented
> on
> the Zeppelin page as an official thing:
> https://zeppelin.apache.org/docs/0.7.0/install/docker.html.
>
> I dunno, it feels weird to me, and I am searching for evidence of these
> issues having been explicitly discussed by other projects but have not
> found it yet.
>
>
>
GPL does not attach by mere aggregation. [see GPL FAQ
]
All linux is gpl, and all the containers are linux for all the other apache
foundation things (maven, httpd, ...). even debian has bash in it, which is
gpl.

so I can either:

a) continue as is. I want to get this on dockerhub auto-built, that's what
he script does now. BTW, it downloads the gpl code from maven repository,
which is also run by apache.
b) remove it, support postgres only.

both are ok w/ me I suppose.

Re: script, GPL, container question

[Gian Merlino]

I found this discussion of Docker images for Apache projects:
https://issues.apache.org/jira/browse/LEGAL-270. It looks like Apache Infra
has been maintaining https://hub.docker.com/u/apache and Apache Legal is
aware of it and didn't see any obvious problems. But I don't see anyone
discussing the question of GPLed components that come from the base image.

On Fri, Jan 25, 2019 at 10:17 AM Gian Merlino  wrote:

> For Q1 the legal guidance as I understand it is that we can provide users
> with instructions for how to get optional (L)GPL dependencies, but we can't
> distribute them ourselves. Putting the mysql-connector in an Docker image
> does feel like distribution…
>
> Q2 is an interesting question. I wonder if Apache has a policy on official
> or semiofficial Docker containers that touches on the possibly thorny
> licensing questions. It seems that they do exist for other projects,
> though: https://hub.docker.com/u/apache. The Zeppelin one, for example,
> is based on ubuntu so it must have plenty of GPL stuff in it:
> https://hub.docker.com/r/apache/zeppelin/dockerfile. And it is presented
> on the Zeppelin page as an official thing:
> https://zeppelin.apache.org/docs/0.7.0/install/docker.html.
>
> I dunno, it feels weird to me, and I am searching for evidence of these
> issues having been explicitly discussed by other projects but have not
> found it yet.
>
> On Wed, Jan 23, 2019 at 3:13 PM Don Bowman  wrote:
>
>> Re: PR https://github.com/apache/incubator-druid/pull/6896
>>
>> I am trying to make a container that works for folks so that we can get
>> the
>> Kubernetes helm chart off the ground.
>>
>> A question has arisen in the PR around the 'mysql-connector-java' which is
>> GPL.
>>
>> Q1. A script checked into the druid container build repo that is Apache
>> license, does anyone have a concern if it has a line 'wget ...
>> mysql-connector.java' [link
>> <
>> https://github.com/apache/incubator-druid/pull/6896#pullrequestreview-195673343
>> >
>> to question]
>> Q2. Given that the container ultimately has GPL things in it (e.g. bash)
>> is
>> there a problem releasing it? [link
>> <
>> https://github.com/apache/incubator-druid/pull/6896#discussion_r250316937
>> >
>> to Q]
>>
>> I'll work on the other comments in the PR.
>>
>> --don
>>
>

Re: script, GPL, container question

[Gian Merlino]

For Q1 the legal guidance as I understand it is that we can provide users
with instructions for how to get optional (L)GPL dependencies, but we can't
distribute them ourselves. Putting the mysql-connector in an Docker image
does feel like distribution…

Q2 is an interesting question. I wonder if Apache has a policy on official
or semiofficial Docker containers that touches on the possibly thorny
licensing questions. It seems that they do exist for other projects,
though: https://hub.docker.com/u/apache. The Zeppelin one, for example, is
based on ubuntu so it must have plenty of GPL stuff in it:
https://hub.docker.com/r/apache/zeppelin/dockerfile. And it is presented on
the Zeppelin page as an official thing:
https://zeppelin.apache.org/docs/0.7.0/install/docker.html.

I dunno, it feels weird to me, and I am searching for evidence of these
issues having been explicitly discussed by other projects but have not
found it yet.

On Wed, Jan 23, 2019 at 3:13 PM Don Bowman  wrote:

> Re: PR https://github.com/apache/incubator-druid/pull/6896
>
> I am trying to make a container that works for folks so that we can get the
> Kubernetes helm chart off the ground.
>
> A question has arisen in the PR around the 'mysql-connector-java' which is
> GPL.
>
> Q1. A script checked into the druid container build repo that is Apache
> license, does anyone have a concern if it has a line 'wget ...
> mysql-connector.java' [link
> <
> https://github.com/apache/incubator-druid/pull/6896#pullrequestreview-195673343
> >
> to question]
> Q2. Given that the container ultimately has GPL things in it (e.g. bash) is
> there a problem releasing it? [link
>  >
> to Q]
>
> I'll work on the other comments in the PR.
>
> --don
>

Re: Indexing Arbitrary Key/Value Data

[Gian Merlino]

Hey Furkan,

There isn't currently an out of the box parser in Druid that can do what
you are describing. But it is an interesting feature to think about. Today
you could implement this using a custom parser (instead of using the
builtin json/avro/etc parsers, write an extension that implements an
InputRowParser, and you can do anything you want, including automatic
flattening of nested data).

In terms of how this might be done out of the box in the future I could
think of a few ideas.

1) Have some way to define an "automatic flatten spec". Maybe something
that systematically flattens in a particular way: in your example, perhaps
it'd automatically create fields like "world.0.hey" and "world.1.tree".

2) A repetition and definition level scheme similar to Parquet:
https://blog.twitter.com/engineering/en_us/a/2013/dremel-made-simple-with-parquet.html.
It sounds like this could be more natural and lend itself to better
compression than (1).

3) Create a new column type designed to store json-like data, although
presumably in some more optimized form. Add some query-time functionality
for extracting values from it. Use this for storing the original input
data. This would only really make sense if you had rollup disabled. In this
case, the idea would be that you would store an entire ingested object in
this new kind of column, and extract some subset fields for faster access
into traditional dimension and metric columns.

On Wed, Jan 9, 2019 at 8:08 AM Furkan KAMACI  wrote:

> Hi Dylan,
>
> Indexing such data as flatten works for my case. I've checked that
> documentation before and this is similar to my need at documentation:
>
> "world": [{"hey": "there"}, {"tree": "apple"}]
>
> However, I don't know what will be the keys at indexing time. Such
> configuration is handled via this at documentation:
>
> ...
> {
>   "type": "path",
>   "name": "world-hey",
>   "expr": "$.world[0].hey"
> },
> {
>   "type": "path",
>   "name": "worldtree",
>   "expr": "$.world[1].tree"
> }
> ...
>
> However, I can't define parseSpec for it as far as I know. I need something
> like working as schemaless or defining a RegEx i.e?
>
> Kind Regards,
> Furkan KAMACI
>
> On Wed, Jan 9, 2019 at 5:45 PM Dylan Wylie  wrote:
>
> > Hey Furkan,
> >
> > Druid can index flat arrays (multi-value dimensions) but not arrays of
> > objects. There is the ability to flatten objects on ingestion using
> > JSONPath. See http://druid.io/docs/latest/ingestion/flatten-json
> >
> > Best regards,
> > Dylan
> >
> > On Wed, 9 Jan 2019 at 14:23, Furkan KAMACI 
> wrote:
> >
> > > Hi All,
> > >
> > > I can index such data with Druid:
> > >
> > > {"ts":"2018-01-01T02:35:45Z","appToken":"guid", "eventName":"app-open",
> > > "key1":"value1"}
> > >
> > > via this configuration:
> > >
> > > "parser" : {
> > > "type" : "string",
> > > "parseSpec" : {
> > >   "format" : "json",
> > >   "timestampSpec" : {
> > > "format" : "iso",
> > > "column" : "ts"
> > >   },
> > >   "dimensionsSpec" : {
> > > "dimensions": [
> > >   "appToken",
> > >   "eventName",
> > >   "key1"
> > > ]
> > >   }
> > > }
> > >   }
> > >
> > > However, I would also want to index such data:
> > >
> > > {
> > >   "ts":"2018-01-01T03:35:45Z",
> > >   "appToken":"guid",
> > >   "eventName":"app-open",
> > >   "properties":[{"randomKey1":"randomValue1"},
> > > {"randomKey2":"randomValue2"}]
> > > }
> > >
> > > at which properties is an array and members of that array has some
> > > arbitrary keys and values.
> > >
> > > How can I do that?
> > >
> > > Kind Regards,
> > > Furkan KAMACI
> > >
> >
>

Re: HAS ISSUE

[Gian Merlino]

Hey Mingwen,

This looks like it's related to the Hive/Druid integration, so it might be
a better question for the Hive mailing list. (The code for that integration
lives in the Hive project.)

On Tue, Jan 22, 2019 at 11:29 PM mingwen@analyticservice.net <
mingwen@analyticservice.net> wrote:

> HI ,
>
> CREATE EXTERNAL TABLE druid_table_1
> STORED BY 'org.apache.hadoop.hive.druid.DruidStorageHandler'
> TBLPROPERTIES ("druid.datasource" = "wikipedia");
>
> BELOW ISSUE,PLEASE HELP ME.THANKS.
> Exception in thread "main" java.lang.NoClassDefFoundError:
> org/apache/hadoop/hive/metastore/DefaultHiveMetaHook
> at java.lang.ClassLoader.defineClass1(Native Method)
> at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
> at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
> at java.net.URLClassLoader.defineClass(URLClassLoader.java:467)
> at java.net.URLClassLoader.access$100(URLClassLoader.java:73)
> at java.net.URLClassLoader$1.run(URLClassLoader.java:368)
> at java.net.URLClassLoader$1.run(URLClassLoader.java:362)
> at java.security.AccessController.doPrivileged(Native Method)
> at java.net.URLClassLoader.findClass(URLClassLoader.java:361)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
> at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:411)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
> at java.lang.Class.forName0(Native Method)
> at java.lang.Class.forName(Class.java:348)
> at
> org.apache.hadoop.hive.ql.parse.ParseUtils.ensureClassExists(ParseUtils.java:225)
> at
> org.apache.hadoop.hive.ql.parse.StorageFormat.fillStorageFormat(StorageFormat.java:64)
> at
> org.apache.hadoop.hive.ql.parse.SemanticAnalyzer.analyzeCreateTable(SemanticAnalyzer.java:10906)
> at
> org.apache.hadoop.hive.ql.parse.SemanticAnalyzer.genResolvedParseTree(SemanticAnalyzer.java:10144)
> at
> org.apache.hadoop.hive.ql.parse.SemanticAnalyzer.analyzeInternal(SemanticAnalyzer.java:10225)
> at
> org.apache.hadoop.hive.ql.parse.SemanticAnalyzer.analyzeInternal(SemanticAnalyzer.java:10110)
> at
> org.apache.hadoop.hive.ql.parse.BaseSemanticAnalyzer.analyze(BaseSemanticAnalyzer.java:223)
> at org.apache.hadoop.hive.ql.Driver.compile(Driver.java:560)
> at org.apache.hadoop.hive.ql.Driver.compileInternal(Driver.java:1358)
> at org.apache.hadoop.hive.ql.Driver.runInternal(Driver.java:1475)
> at org.apache.hadoop.hive.ql.Driver.run(Driver.java:1287)
> at org.apache.hadoop.hive.ql.Driver.run(Driver.java:1277)
> at org.apache.hadoop.hive.cli.CliDriver.processLocalCmd(CliDriver.java:226)
> at org.apache.hadoop.hive.cli.CliDriver.processCmd(CliDriver.java:175)
> at org.apache.hadoop.hive.cli.CliDriver.processLine(CliDriver.java:389)
> at org.apache.hadoop.hive.cli.CliDriver.executeDriver(CliDriver.java:781)
> at org.apache.hadoop.hive.cli.CliDriver.run(CliDriver.java:699)
> at org.apache.hadoop.hive.cli.CliDriver.main(CliDriver.java:634)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at org.apache.hadoop.util.RunJar.run(RunJar.java:226)
> at org.apache.hadoop.util.RunJar.main(RunJar.java:141)
> Caused by: java.lang.ClassNotFoundException:
> org.apache.hadoop.hive.metastore.DefaultHiveMetaHook
> at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
> at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
>
>
>
>
>
> mingwen@analyticservice.net
>

Re: The etiquette of pocking people on Github and the policy when people stop responding

[Gian Merlino]

If enough other committers have already reviewed and accepted a patch, I
don't think it's fair to the author or to those other reviewers for
committing to be delayed by two weeks because another committer doesn't
have time to finish reviewing, but wants others to wait for them anyway. A
couple of days, sure, but two weeks is quite a long time. It would be
potentially longer in practice, since with your proposal the two-week clock
would start fresh if the reviewer had some more follow-up comments.

Presumably the not-yet-finished reviewer's motivation for wanting others to
wait for them is that they have some set of concerns that they feel other
reviewers may not be examining. IMO, it'd be better to ask the reviewers
that do have more time to take those concerns into account rather than
blocking the commit.

And of course - even after patches are committed, we still have release
votes and the concept of release blocker issues as a final gate to allow
people to express an opinion that particular code should not be released
without changes. So the commit of a patch itself is not the only gate that
exists before code is released.

On Fri, Jan 25, 2019 at 3:43 AM Roman Leventov 
wrote:

> The times that I suggested are IMO minimally reasonable for not provoking a
> sense of rush and anxiety in people being poked.
>
> If we assume that people adhere to the guideline "do not comment on a pull
> request unless you are willing to follow up on the edits" from here:
>
> https://github.com/druid-io/druid-io.github.io/blob/src/community/index.md#general-committer-guidelines
> and don't forget about the PRs they started reviewing, poking appears to be
> pointless. Poking and expecting the reviewer to respond "I didn't forget
> about it, I'll continue my review soon" is not a good use of the time of
> both people and IMO should be the common practice.
>
> Proposal reviews are good but the code should be reviewed thoroughly too.
> Despite the proposal got enough approvals, the PR with the actual code
> shouldn't be rushed into the codebase.
>
> On Fri, 25 Jan 2019 at 04:05, Gian Merlino  wrote:
>
> > The timelines you outlined seem quite slow. Especially "if there are
> enough
> > approvals, a PR could be merged not sooner than in two weeks since they
> > left the last review comment". IMO, rather than delaying patches by so
> > long, a better way to be courteous of a reviewer being too busy to review
> > in a timely manner is to seek review from some other committer that has
> > more time.
> >
> > In an extreme case, where the patch has issues that a reviewer feels must
> > be addressed before the patch is merged, but the reviewer does not have
> > time to hold up their end of that conversation, they can veto (
> > https://www.apache.org/foundation/voting.html#Veto), accompanied by a
> > justification. This is a pretty blunt tool and should not be used much.
> But
> > it is there.
> >
> > I'm still optimistic that the discussion we've been having around
> proposals
> > is a good way to address a desire for reviewers to have their say, in a
> way
> > that doesn't slow down the development process so much. By starting
> > conversations about changes earlier, it is a way for contributors to come
> > together and agree on the general shape of changes before there is a
> bunch
> > of code to discuss. Hopefully that also makes code review more efficient
> in
> > terms of contributors' time, reviewers' time, and amount of calendar days
> > that PRs are open for.
> >
> > On Thu, Jan 24, 2019 at 3:41 AM Roman Leventov 
> > wrote:
> >
> > > To foster calmness, respect, and consideration of people's busy
> > schedules I
> > > suggest the following etiquette:
> > >
> > > - When someone showed up in a PR and left some review comments, but
> > didn't
> > > explicitly approved the PR, poke them with comments like "@username do
> > you
> > > have more comments?" not sooner than in one week since they left the
> last
> > > review comment.
> > > - Poke people no more frequently than once per week.
> > > - If there are enough approvals, a PR could be merged not sooner than
> in
> > > two weeks since they left the last review comment.
> > > - If someone created a valuable PR but then stopped responding to
> review
> > > comments and there are conflicts or tests/CI don't pass, a PR could be
> > > recreated by another person not sooner than in three weeks since the
> > > author's last activity in the PR. Their authorship should be preserved
> by
> > > cherry-picking their commits, squashing them locally, rebasing on top
> of
> > > current upstream master, creating a new PR and choosing "Rebase and
> > merge"
> > > option when merging the new PR instead of the default "Squash and
> merge".
> > >
> >
>

Re: The etiquette of pocking people on Github and the policy when people stop responding

[Roman Leventov]

The times that I suggested are IMO minimally reasonable for not provoking a
sense of rush and anxiety in people being poked.

If we assume that people adhere to the guideline "do not comment on a pull
request unless you are willing to follow up on the edits" from here:
https://github.com/druid-io/druid-io.github.io/blob/src/community/index.md#general-committer-guidelines
and don't forget about the PRs they started reviewing, poking appears to be
pointless. Poking and expecting the reviewer to respond "I didn't forget
about it, I'll continue my review soon" is not a good use of the time of
both people and IMO should be the common practice.

Proposal reviews are good but the code should be reviewed thoroughly too.
Despite the proposal got enough approvals, the PR with the actual code
shouldn't be rushed into the codebase.

On Fri, 25 Jan 2019 at 04:05, Gian Merlino  wrote:

> The timelines you outlined seem quite slow. Especially "if there are enough
> approvals, a PR could be merged not sooner than in two weeks since they
> left the last review comment". IMO, rather than delaying patches by so
> long, a better way to be courteous of a reviewer being too busy to review
> in a timely manner is to seek review from some other committer that has
> more time.
>
> In an extreme case, where the patch has issues that a reviewer feels must
> be addressed before the patch is merged, but the reviewer does not have
> time to hold up their end of that conversation, they can veto (
> https://www.apache.org/foundation/voting.html#Veto), accompanied by a
> justification. This is a pretty blunt tool and should not be used much. But
> it is there.
>
> I'm still optimistic that the discussion we've been having around proposals
> is a good way to address a desire for reviewers to have their say, in a way
> that doesn't slow down the development process so much. By starting
> conversations about changes earlier, it is a way for contributors to come
> together and agree on the general shape of changes before there is a bunch
> of code to discuss. Hopefully that also makes code review more efficient in
> terms of contributors' time, reviewers' time, and amount of calendar days
> that PRs are open for.
>
> On Thu, Jan 24, 2019 at 3:41 AM Roman Leventov 
> wrote:
>
> > To foster calmness, respect, and consideration of people's busy
> schedules I
> > suggest the following etiquette:
> >
> > - When someone showed up in a PR and left some review comments, but
> didn't
> > explicitly approved the PR, poke them with comments like "@username do
> you
> > have more comments?" not sooner than in one week since they left the last
> > review comment.
> > - Poke people no more frequently than once per week.
> > - If there are enough approvals, a PR could be merged not sooner than in
> > two weeks since they left the last review comment.
> > - If someone created a valuable PR but then stopped responding to review
> > comments and there are conflicts or tests/CI don't pass, a PR could be
> > recreated by another person not sooner than in three weeks since the
> > author's last activity in the PR. Their authorship should be preserved by
> > cherry-picking their commits, squashing them locally, rebasing on top of
> > current upstream master, creating a new PR and choosing "Rebase and
> merge"
> > option when merging the new PR instead of the default "Squash and merge".
> >
>