Seems reasonable to have a way to make it easy for people to follow important
things for people who are not glued to the mailing lists.
If the desire is to keep it light weight and still achieve the above goal ..
the SIP can be just a short title and description with a link to the JIRA
which c
Arun,
For big features we did follow design doc/review. Making it
formal makes everyone to follow a process.
Again this process is not for bug fixes as we stated its about New
Features/Config Changes/Public interface changes. I don't think it puts
any extra effort for anyone
Github user askprasanna commented on the issue:
https://github.com/apache/storm/pull/2152
I picked the existing JIRA since it is a followup to the same issue and
addresses something that was missed in the earlier commits. Can we not reopen
and re-resolved?
---
If your project is set
Github user askprasanna commented on a diff in the pull request:
https://github.com/apache/storm/pull/2152#discussion_r121249114
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -104,6 +104,10 @@ public KafkaSpout(KafkaSpoutCo
Dynamic assignment is what causing all the issues that we see now.
1. Duplicates at the start of the KafkaSpout and upon any rebalance
2. Trident Kafka Spout not holding the transactional batches.
Many corner cases can easily produce duplicates.
There is no point in keeping dynamic assignment giv
I am for documenting and upfront design reviews, but maybe we should keep it
less formal and make it part of the JIRA to start with.
Do we have any upcoming features for which we would like to see a proposal? May
be start with a couple of proposals
and see it works out before making it formal.
-0
The KIP process feels kind of heavy. I'd rather start with a lighter effort
like improving JIRA submissions and pull requests (some pull requests/JIRAs,
even from committers and PMC members, are woefully inadequate in terms of
detail), and see how that works out.
I share Bobby's concern tha
+1 for SIPs including a new connector. The person writing SIP can provide
details about the external system for which connector is being written to let
others know why a certain design decision was made. This will make it easy for
reviewers.
On 6/9/17, 5:24 PM, "Satish Duggana" wrote:
+1
Just realized that from my email it was not clear what were my comments versus
Stig’s. I have put mine in [] below.
On 6/9/17, 5:42 PM, "Priyank Shah" wrote:
I want to add a few things other than the issues raised by Sriharsha and
Hugo. I am pasting one of the other emails that I sent some
I want to add a few things other than the issues raised by Sriharsha and Hugo.
I am pasting one of the other emails that I sent sometime back about cleaning
up KafkaSpoutConfig. Stig responsed to that email. Trying to answer that in
this email so that it is all in one place. Answers in line.
+1 for SIPs. It is so useful as mentioned by others in earlier mails. It
would be very useful for new contributors and others who are looking out
for a feature design and decisions taken etc.
Whenever a minor feature is added to a connector it may not need a separate
SIP but the existing README ca
If I am looking at the Kafka site correctly, I see that Kafka has a total of
167 KIPs so far.
So I assume that minor new features would not be parrt of the SIP ?
Unlike Kafka, since Storm has a number of connectors (that keep growing), I am
speculating the SIP process might get somewhat unwiel
+1 for KIP equivalent for Storm. We should probably call them something like
Storm Feature/Improvement Proposals.
Well documented features could help new contributors to more quickly as they
would have the context to start that feature. Unless one knows the product
well, it is hard to have feat
+1 for simplifying KafkaSpoutConfig. Too many constructors and too many
methods.. I am not sure it’s justifiable to have any methods that simply set
KafkaConsumer properties. All of these properties should just go in a
Map, which is what KafkaConsumer receives, and what was
supported in the ini
I think question why we need all those settings when a user can pass it
via Properties with consumer properties defined or via Map conf object.
Having the methods on top of consumer config means every time Kafka
consumer property added or changed one needs add a builder method. We
need to get out
Hi Bobby,
In general, a KIP is required for adding New features, config
changes or backward-incompatible changes. Don't require
adding a KIP for bug-fixes. Devs who wants to add any
features will write up a wiki which has JIRA link, mailing
li
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2152
I'm not sure if we should have a new JIRA issue for this btw. The issue
this links to is already marked fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appe
Can you please explain how KIP currently works and how you would like to see
something similar in storm?
If we make the process more formal we will probably have less people
contributing, but we will probably have better overall patches. It is a
balancing act and having never used KIP I would l
This sounds like a good idea. KIPs seem to work well for Kafka. It's easy
for discussions to get lost or just not seen on the mailing list.
2017-06-09 21:36 GMT+02:00 Harsha :
> Hi All,
> We’ve seen good adoption of KIP approach in Kafka community
> and would like to see we ad
I'd be happy with a simpler KafkaSpoutConfig, but I think most of the
configuration parameters have good reasons for being there. Any examples of
parameters you think we should remove?
2017-06-09 21:34 GMT+02:00 Harsha :
> +1 on using the manual assignment for the reasons specified below. We
> w
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2152#discussion_r121217733
--- Diff:
external/storm-kafka-client/src/test/java/org/apache/storm/kafka/spout/KafkaSpoutCommitTest.java
---
@@ -0,0 +1,145 @@
+/*
+ * Copyright 2017
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2152#discussion_r121218000
--- Diff:
external/storm-kafka-client/src/test/java/org/apache/storm/kafka/spout/KafkaSpoutCommitTest.java
---
@@ -0,0 +1,145 @@
+/*
+ * Copyright 2017
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2152#discussion_r121218037
--- Diff:
external/storm-kafka-client/src/test/java/org/apache/storm/kafka/spout/KafkaSpoutCommitTest.java
---
@@ -0,0 +1,145 @@
+/*
+ * Copyright 2017
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2152#discussion_r121221199
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -104,6 +104,10 @@ public KafkaSpout(KafkaSpoutConfig
k
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2095
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enab
Github user askprasanna commented on the issue:
https://github.com/apache/storm/pull/2152
Added the unit test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2117
@srdo thought I sent and replied your earlier emails as well. Looks like
some issue with gmail they are showing as sent but didn't reached the mailing
list. I sent them again.
---
If your project
Hi All,
We’ve seen good adoption of KIP approach in Kafka community
and would like to see we adopt the similar approach for storm
as well.
Its hard to keep track of proposed changes and mailing list threads to
know what all changes that are coming into and what design
+1 on using the manual assignment for the reasons specified below. We
will see duplicates even in stable conditions which
is not good. I don’t see any reason not to switch to manual assignment.
While we are at it we should refactor the KafkaConfig part.
It should be as simple as accepting the kaf
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2153#discussion_r121163277
--- Diff:
external/storm-kafka-client/src/test/java/org/apache/storm/kafka/spout/KafkaSpoutRetryLimitTest.java
---
@@ -0,0 +1,126 @@
+/*
+ * Copyright
Github user askprasanna commented on the issue:
https://github.com/apache/storm/pull/2153
Have added the unit test as suggested. Kindly review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user liu-zhaokun commented on the issue:
https://github.com/apache/storm/pull/2110
@HeartSaVioR
The explanation has existed in docs\SECURITY.md,so I didn't add it in these
files.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2110
@liu-zhaokun
Then it would be better to document that to make intention clearer. Could
you please add explanation
above the example?
---
If your project is set up for it, you can reply
Github user liu-zhaokun commented on the issue:
https://github.com/apache/storm/pull/2110
@HeartSaVioR
kishorvpatil approved these changes 28 days ago,could you help me merge
this PR as soon as possible?
---
If your project is set up for it, you can reply to this email and have
Github user liu-zhaokun commented on the issue:
https://github.com/apache/storm/pull/2110
@HeartSaVioR
Hello,long time no see!Anyway,thanks for your reply.
The $ here should be replace with their keytab and principal by
users manually.
---
If your project is set up for it, y
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2110
@liu-zhaokun Sorry to visit too late. Btw I'm not expert on this and also
not sure what `$` came from. Do users need to replace it manually, or
that's reserved variables?
---
If your project is
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2139
@Angus-Slalom
Sorry to visit this too later. This looks good. +1
Could you squash your commits into one? I recommend adding `STORM-2484` to
the commit title to help tracking later.
---
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2119
@ryanpersaud Thanks for contributing. We introduced checkstyle and the
build is failing with checkstyle violation. Could you fix the violation from
your changeset and update the patch?
---
If y
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2128#discussion_r121072928
--- Diff:
external/storm-elasticsearch/src/main/java/org/apache/storm/elasticsearch/bolt/EsPercolateBolt.java
---
@@ -81,7 +81,7 @@ public void process(
39 matches
Mail list logo
