UPDATES: I've filed an issue STORM-2453 [1] and submitted a PR [2]. Please review so that we can finalize this discussion.
Thanks, Jungtaek Lim (HeartSaVioR) [1] https://issues.apache.org/jira/browse/STORM-2453 [2] https://github.com/apache/storm/pull/2042 2017년 3월 31일 (금) 오후 3:40, Xin Wang <data.xinw...@gmail.com>님이 작성: Jungtaek, Currently no patches, the work is in progress. I will submit several days later. 2017-03-31 14:18 GMT+08:00 Jungtaek Lim <kabh...@gmail.com>: > Xin, > > Do you already have patches? Or just waiting for this before doing some > works? > > 2017년 3월 31일 (금) 오후 3:10, Xin Wang <data.xinw...@gmail.com>님이 작성: > > > +1 on keeping the current external/storm-* in place and move flux, sql, > > storm-submit-tools into top-level. > > > > BTW, I am waiting for the discuss result before submitting several SQL > > related PRs. :) > > > > - Xin > > > > 2017-03-31 10:41 GMT+08:00 Jungtaek Lim <kabh...@gmail.com>: > > > > > Yeah sure I'm OK to just apply it for master branch. > > > Are you okay for moving them to root directory without renaming? Or do > > you > > > want to rename or suggest other base directory? > > > > > > 2017년 3월 31일 (금) 오전 11:36, P. Taylor Goetz <ptgo...@gmail.com>님이 작성: > > > > > > > I'm hesitant to change the layout of the 1.x release. People do some > > > crazy > > > > things when it comes to operations that we can't predict. I'm okay > with > > > > doing this on the master/2.0 branch, but I'm hesitant on the 1.x > > branch. > > > > > > > > -Taylor > > > > > > > > > On Mar 30, 2017, at 9:01 PM, Jungtaek Lim <kabh...@gmail.com> > wrote: > > > > > > > > > > Once we just released Storm 1.1.0, I guess we can continue > discussion > > > > here. > > > > > > > > > > I checked the PR list, and there're no open PRs for among Flux, > Storm > > > > SQL, > > > > > and Storm submit tool. So it's good to go. > > > > > > > > > > I think we have consensus to move non-connectors modules to out of > > > > > external. There're also some interest about renaming "external" to > > > > > "connectors", but given that "external" is chosen by community and > > has > > > > been > > > > > the norm for years, so I agree it would be better not to rename it. > > We > > > > can > > > > > do it later when there's another chance of doing it. > > > > > > > > > > We didn't decide where to move, but most of us seems to be OK to > move > > > to > > > > > the top directory. > > > > > Taylor, any further opinion regarding this? > > > > > Suppose we're moving them to the top directory (or whatever the > same > > > base > > > > > directory), what would be good names for them? Flux doesn't have > > prefix > > > > > 'storm' so a bit different, but if we're OK we skip renaming it. > > > > > > > > > > I'll do the work when Taylor is OK for changing this. > > > > > > > > > > - Jungtaek Lim (HeartSaVioR) > > > > > > > > > > 2017년 3월 27일 (월) 오전 11:16, Jungtaek Lim <kabh...@gmail.com>님이 작성: > > > > > > > > > >> 1. apply versions > > > > >> > > > > >> First plan was applying this only for master, but realized that > > > > >> contributors should make two patches for every PRs when we apply > > this > > > to > > > > >> only master. > > > > >> > > > > >> So in order to make less inconvenience, it would be better to > apply > > > this > > > > >> for both 1.x and master. It also affects opened pull requests so > we > > > > would > > > > >> like to check that relevant PRs are open, and apply it later than > > > > reviewing > > > > >> them. > > > > >> > > > > >> I agree with Harsha. No need to make change for current release > > vote. > > > > >> > > > > >> 2. naming issue for "external" > > > > >> > > > > >> "external" makes me feel that it's related to "external" > component, > > > say, > > > > >> outside of Storm. That's why I suggest moving non-connectors to > out > > of > > > > >> "external". IMHO "connector" is still more intuitive and > > > > self-describing, > > > > >> but I understand that renaming the directory structure would be > > > painful, > > > > >> and "storm-kafka-monitor" is an example of what it's not a > > "connector" > > > > but > > > > >> an "external". So I'm OK to keep it as "external". > > > > >> > > > > >> 3. where to move non-connectors > > > > >> > > > > >> Except Flux they're directly supported by Storm. I mean "storm.py" > > is > > > > >> aware of them and supports them, so for me they are eligible to > move > > > to > > > > the > > > > >> top directory. I'm open to other suggestion as well. > > > > >> > > > > >> - Jungtaek Lim (HeartSaVioR) > > > > >> > > > > >> 2017년 3월 26일 (일) 오후 12:14, Harsha Chintalapani <st...@harsha.io > >님이 > > > 작성: > > > > >> > > > > >> We should this in next release of 1.x or 2.0. I am +1 on continue > > with > > > > >> current release. > > > > >> -Harsha > > > > >>> On Fri, Mar 24, 2017 at 8:53 PM P. Taylor Goetz < > ptgo...@gmail.com > > > > > > > wrote: > > > > >>> > > > > >>> The question remains if we want to do this in the 1.1.0 release, > or > > > > >> later. > > > > >>> > > > > >>> If it's the 1.1.0 release we need to make the changes and cut > > another > > > > RC. > > > > >>> I'm fine with that, but want to make sure we have consensus > before > > > > going > > > > >>> down that road. > > > > >>> > > > > >>> -Taylor > > > > >>> > > > > >>>> On Mar 24, 2017, at 10:57 PM, Harsha Chintalapani < > > st...@harsha.io> > > > > >>> wrote: > > > > >>>> > > > > >>>> Agree on change like this would be confusing to the users. Lets > > keep > > > > >> the > > > > >>>> original plan of moving non-connectors modules of external > instead > > > of > > > > >>>> introducing new changes > > > > >>>> that are not in scope of this discussion. > > > > >>>> My +1 still stands on keeping the current external/storm-* in > > place > > > > and > > > > >>>> move just sql and storm-perf into top-level. We can have > > discussion > > > > for > > > > >>>> storm 2.0 if we want to do > > > > >>>> more changes. > > > > >>>> > > > > >>>> -Harsha > > > > >>>> > > > > >>>>> On Fri, Mar 24, 2017 at 4:31 PM P. Taylor Goetz < > > ptgo...@gmail.com > > > > > > > > >>> wrote: > > > > >>>>> > > > > >>>>> If we decide to change the structure of the distribution like > > > this, I > > > > >>>>> think we should do it in masrwe/2.0. If we want this for 1.1.0 > we > > > > need > > > > >>> to > > > > >>>>> cut a new release candidate. > > > > >>>>> > > > > >>>>> Changing the structure of the distribution file structure can > be > > > > >>>>> disruptive for users. Even the change to no longer include > > > connector > > > > >>>>> binaries, as we've learned, will be a headache for some users. > > > > >>>>> > > > > >>>>> IMHO, from an ops perspective, changes like this should be > > handled > > > > >> like > > > > >>>>> API changes. > > > > >>>>> > > > > >>>>> -Taylor > > > > >>>>> > > > > >>>>>> On Mar 24, 2017, at 4:07 PM, Hugo Da Cruz Louro < > > > > >>> hlo...@hortonworks.com> > > > > >>>>> wrote: > > > > >>>>>> > > > > >>>>>> Another possibility is to keep the ‘external' module, and > create > > > sub > > > > >>>>> modules under it. The legacy structure would remain intact, > while > > > > >> making > > > > >>>>> things more modular. An idea would be: > > > > >>>>>> > > > > >>>>>> + external > > > > >>>>>> + connectors > > > > >>>>>> + tools > > > > >>>>>> + monitoring > > > > >>>>>> + etc > > > > >>>>>> > > > > >>>>>> Hugo > > > > >>>>>> > > > > >>>>>>> On Mar 24, 2017, at 12:34 PM, P. Taylor Goetz < > > ptgo...@gmail.com > > > > > > > > >>>>> wrote: > > > > >>>>>>> > > > > >>>>>>> For the background on why “external” was selected, you have > to > > go > > > > >> back > > > > >>>>> to a lengthy discussion in Feb. 2014. > > > > >>>>>>> > > > > >>>>>>> Here’s the start of the thread: > > > > >>>>>>> > > > > >>>>>>> > > > > >>>>> > > > > >>> > > > > >> > > > > http://mail-archives.apache.org/mod_mbox/storm-dev/201402. > > > mbox/%3cee2bd0e2-254c-47af-8a53-257db7f05...@gmail.com%3e > > > > >>>>> < > > > > >>>>> > > > > >>> > > > > >> > > > > http://mail-archives.apache.org/mod_mbox/storm-dev/201402. > > > mbox/%3cee2bd0e2-254c-47af-8a53-257db7f05...@gmail.com%3E > > > > >>>>>> > > > > >>>>>>> > > > > >>>>>>> It continues into March: > > > > >>>>>>> > > > > >>>>>>> > > > > >>>>> > > > > >>> > > > > >> > > > > http://mail-archives.apache.org/mod_mbox/storm-dev/201403.mbox/% > > > 3ccadimvzum1d3om30zayqq4xxe1vjbn7fumqcsgu+524oqgec...@mail.gmail.com > %3e > > > > >>>>>>> > > > > >>>>>>> I’m -1 on renaming “external”. That’s the name chosen by the > > > > >> community > > > > >>>>> and it has been the norm for 3 years. Changing it would likely > > > > confuse > > > > >>>>> users. > > > > >>>>>>> > > > > >>>>>>> One of the ideas behind “external” was that it would contain > > > > >>> components > > > > >>>>> that were not essential to running storm. That line has > recently > > > > >> blurred > > > > >>>>> with some non-connector code sneaking in, so I’m okay with > moving > > > > >>>>> non-connector code out of external. Another point in that > thread > > > was > > > > a > > > > >>>>> desire to avoid cluttering up the root directory, so we need to > > be > > > > >>> careful > > > > >>>>> about what the destination for those components is. > > > > >>>>>>> > > > > >>>>>>> -Taylor > > > > >>>>>>> > > > > >>>>>>>> On Mar 24, 2017, at 3:11 PM, Hugo Da Cruz Louro < > > > > >>>>> hlo...@hortonworks.com> wrote: > > > > >>>>>>>> > > > > >>>>>>>> +1 non-connectors to top level > > > > >>>>>>>> +1 to renaming external to connectors > > > > >>>>>>>> > > > > >>>>>>>> As for storm-kaka, if we are already touching the external > > > > modules, > > > > >>>>> all the modules should be a submodule of a parent module called > > > > >>>>> storm-kafka. I don’t think we should have 3 parent modules as > we > > > > >>> currently > > > > >>>>> have (storm-kafka, storm-kafka-client, storm-kafka-monitor) > > > > >>>>>>>> > > > > >>>>>>>> The structure should be something along the lines (I don’t > > mean > > > > the > > > > >>>>> exact names; we should find better ones. storm-kafka and > > > > >>>>> storm-kafka-client are not very self explanatory in my opinion) > > > > >>>>>>>> > > > > >>>>>>>> + storm-kafka > > > > >>>>>>>> + monitoring > > > > >>>>>>>> + new-client > > > > >>>>>>>> + old-client > > > > >>>>>>>> > > > > >>>>>>>> If we have to create new modules or submodules (e.g. under > > > utils) > > > > >> so > > > > >>>>> be it. The code should be in a module that is named after what > > its > > > > >>> doing. > > > > >>>>>>>> > > > > >>>>>>>> Hugo > > > > >>>>>>>> > > > > >>>>>>>>> On Mar 24, 2017, at 11:15 AM, Priyank Shah < > > > > ps...@hortonworks.com > > > > >>> > > > > >>>>> wrote: > > > > >>>>>>>>> > > > > >>>>>>>>> +1 to moving non-conncectors to top level. I think we > should > > > keep > > > > >>>>> stom-kafka-monitor under external or connectors(after > renaming). > > > > >>>>>>>>> > > > > >>>>>>>>> Jungtaek, just to clarify on what you said regarding storm > > core > > > > >>>>> referencing storm-kafka-monitor. Like you said its just calling > > the > > > > >>> script > > > > >>>>> from ui jvm. There is no dependency in terms of class files > > needed > > > to > > > > >>> run > > > > >>>>> the script from ui. The script itself adds a –cp argument and > all > > > it > > > > >>> needs > > > > >>>>> is storm-kafka-monitor jar in classpath. As far as packaging > the > > > > >> script > > > > >>> is > > > > >>>>> concerned we can do what Satish suggested. i.e. move it to > > > > >>>>> storm-kafka-monitor in source and while packaging put it under > > bin. > > > > >>>>> Reiterating to make sure I am not mis-understanding anything. > > > > >>>>>>>>> > > > > >>>>>>>>> On 3/24/17, 9:14 AM, "Harsha Chintalapani" < > st...@harsha.io> > > > > >> wrote: > > > > >>>>>>>>> > > > > >>>>>>>>> +1 on moving non-connectors to top-level like sql and > > > storm-perf. > > > > >>>>>>>>> Regarding storm-kafka-monitor we can move this into "util" > > > folder > > > > >> or > > > > >>>>> keep > > > > >>>>>>>>> in the external. > > > > >>>>>>>>> -Harsha > > > > >>>>>>>>> > > > > >>>>>>>>> On Fri, Mar 24, 2017 at 2:23 AM Satish Duggana < > > > > >>>>> satish.dugg...@gmail.com> > > > > >>>>>>>>> wrote: > > > > >>>>>>>>> > > > > >>>>>>>>>> storm-kafka-monitor is not a connector by itself but it is > > > > >> related > > > > >>>>> to kafka > > > > >>>>>>>>>> connectors. So, any utility related to that connector > should > > > be > > > > >>> part > > > > >>>>> of > > > > >>>>>>>>>> that connector module(can be a submodule) instead of a top > > > level > > > > >>>>> module. > > > > >>>>>>>>>> core/ui uses this utility referring directly in a hacky > way, > > > > >> which > > > > >>>>> we may > > > > >>>>>>>>>> want to fix later. storm-kafka-monitor script exists in > bin > > > > >>>>> directory which > > > > >>>>>>>>>> can be moved to storm-kafka-monitor module and the same > > script > > > > >> can > > > > >>> be > > > > >>>>>>>>>> packaged as part of storm/bin directory while packaging > the > > > > >>>>> distribution. > > > > >>>>>>>>>> > > > > >>>>>>>>>> Thanks, > > > > >>>>>>>>>> ~Satish. > > > > >>>>>>>>>> > > > > >>>>>>>>>>> On Fri, Mar 24, 2017 at 1:07 PM, Jungtaek Lim < > > > > >> kabh...@gmail.com> > > > > >>>>> wrote: > > > > >>>>>>>>>>> > > > > >>>>>>>>>>> storm-kafka-monitor is referred by storm-core, though > it's > > > > >>>>> referenced via > > > > >>>>>>>>>>> executing command. Yes it's a bit odd to place it as top > > > > >>> directory, > > > > >>>>> but > > > > >>>>>>>>>>> it's not a connector for that reason too. Neither is > ideal > > > for > > > > >> me, > > > > >>>>> so > > > > >>>>>>>>>>> ironically, either is fine. > > > > >>>>>>>>>>> > > > > >>>>>>>>>>> - Jungtaek Lim (HeartSaVioR) > > > > >>>>>>>>>>> > > > > >>>>>>>>>>> 2017년 3월 24일 (금) 오후 4:19, Satish Duggana < > > > > >>> satish.dugg...@gmail.com > > > > >>>>>> 님이 > > > > >>>>>>>>>> 작성: > > > > >>>>>>>>>>> > > > > >>>>>>>>>>>> +1 except for storm-kafka-monitor module as this utility > > is > > > > >> more > > > > >>>>> about > > > > >>>>>>>>>>>> querying topic/partition offsets of kafka spouts in a > > > > topology. > > > > >>> Do > > > > >>>>> not > > > > >>>>>>>>>> we > > > > >>>>>>>>>>>> want to push this module into connectors/kafka as a > > > submodule > > > > >>> along > > > > >>>>>>>>>> with > > > > >>>>>>>>>>>> other submodules including old/new kafka spout modules? > > > > >>>>>>>>>>>> > > > > >>>>>>>>>>>> Thanks, > > > > >>>>>>>>>>>> Satish. > > > > >>>>>>>>>>>> > > > > >>>>>>>>>>>> > > > > >>>>>>>>>>>> On Fri, Mar 24, 2017 at 12:10 PM, Arun Iyer < > > > > >>> ai...@hortonworks.com > > > > >>>>>> > > > > >>>>>>>>>>> wrote: > > > > >>>>>>>>>>>> > > > > >>>>>>>>>>>>> +1 > > > > >>>>>>>>>>>>> > > > > >>>>>>>>>>>>> Makes sense to move the non-connectors to top level and > > > keep > > > > >>> only > > > > >>>>> the > > > > >>>>>>>>>>>>> connectors under “connectors” folder. > > > > >>>>>>>>>>>>> > > > > >>>>>>>>>>>>> > > > > >>>>>>>>>>>>>> On 3/24/17, 12:00 PM, "Jungtaek Lim" < > kabh...@gmail.com > > > > > > > >>> wrote: > > > > >>>>>>>>>>>>>> > > > > >>>>>>>>>>>>>> (Sent this yesterday but can't find this from > storm-dev > > > > >> mbox... > > > > >>>>>>>>>>> sending > > > > >>>>>>>>>>>> it > > > > >>>>>>>>>>>>>> again) > > > > >>>>>>>>>>>>>> > > > > >>>>>>>>>>>>>> Hi dev, > > > > >>>>>>>>>>>>>> > > > > >>>>>>>>>>>>>> I'd like to start discussion regarding moving > > > non-connectors > > > > >>>>> modules > > > > >>>>>>>>>>> out > > > > >>>>>>>>>>>>> of > > > > >>>>>>>>>>>>>> external, maybe top directory. > > > > >>>>>>>>>>>>>> > > > > >>>>>>>>>>>>>> "external" directory has non-connectors (SQL, Flux, > > > > >>>>>>>>>>> storm-kafka-monitor, > > > > >>>>>>>>>>>>>> storm-submit-tools), and except Flux, others should be > > > > placed > > > > >>> to > > > > >>>>> the > > > > >>>>>>>>>>>>> binary > > > > >>>>>>>>>>>>>> dist. since Storm itself (not from user topology) > needs > > to > > > > >>> refer > > > > >>>>>>>>>> them. > > > > >>>>>>>>>>>>>> > > > > >>>>>>>>>>>>>> They're actually tied to the core of Storm, so I feel > > that > > > > it > > > > >>>>> would > > > > >>>>>>>>>> be > > > > >>>>>>>>>>>>>> better to treat them (including Flux) as non-external, > > > maybe > > > > >>> same > > > > >>>>>>>>>>> level > > > > >>>>>>>>>>>> as > > > > >>>>>>>>>>>>>> storm-core. > > > > >>>>>>>>>>>>>> (I'm not sure what "external" actually means for Storm > > > > >> project > > > > >>>>> btw.) > > > > >>>>>>>>>>>>>> > > > > >>>>>>>>>>>>>> In addition, after doing that I'd like to change the > > > > >> directory > > > > >>>>> name > > > > >>>>>>>>>>>>>> "external" to "connector" or so, so that the name > could > > be > > > > >>>>>>>>>>>> self-describing > > > > >>>>>>>>>>>>>> and we can only place connectors to that directory. > > > > >>>>>>>>>>>>>> (I know it would be painful for already opened pull > > > > requests, > > > > >>> so > > > > >>>>> no > > > > >>>>>>>>>>>> strong > > > > >>>>>>>>>>>>>> opinion regarding this.) > > > > >>>>>>>>>>>>>> > > > > >>>>>>>>>>>>>> Looking forward to your opinion! > > > > >>>>>>>>>>>>>> > > > > >>>>>>>>>>>>>> Thanks, > > > > >>>>>>>>>>>>>> Jungtaek Lim (HeartSaVioR) > > > > >>>>>>>>>>>>> > > > > >>>>>>>>>>>>> > > > > >>>>>>>>>>>> > > > > >>>>>>>>>>> > > > > >>>>>>>>>> > > > > >>>>>>>>> > > > > >>>>>>>>> > > > > >>>>>>>> > > > > >>>>>>> > > > > >>>>>> > > > > >>>>> > > > > >>> > > > > >> > > > > >> > > > > > > > > > >
