> As a separate issue, I will also volunteer to see if I can help Tamas find the discuss thread mentioned. It should be linked to the PR or feature branch for reference. That may also be a gap in dev guidelines that should be spelled out.
Just following up on this. I checked and verified we have existing recommendations around discussion threads for new features, so no immediate tasks to tackle there - https://cwiki.apache.org/confluence/display/METRON/Development+Guidelines " 1.1 Contributing A Code Change ... If you are introducing a completely new feature or API it is a good idea to start a discussion and get consensus on the basic design first. Larger changes should be discussed on the dev boards before submission. New features and significant bug fixes should be documented in the JIRA and appropriate architecture diagrams should be attached. Major features may require a vote. Note that if the change is related to user-facing protocols / interface / configs, etc, you need to make the corresponding change on the documentation as well. " Per the new DISCUSS thread and Jira updates from Shane ( https://lists.apache.org/thread.html/ad206bdd59594cf74560770dfdbfcde0addd120d6fa8ea73f1a92a6b@%3Cdev.metron.apache.org%3E) it looks like we're in good shape for referencing past discussions for this feature and have any remaining gaps being covered. On Thu, May 2, 2019 at 7:31 AM Michael Miklavcic < michael.miklav...@gmail.com> wrote: > I am still in favor of option 2. I will volunteer and submit the doc PR. I > agree we should not rush through a review process for a maintenance > release. The implications to the UI, as Otto asked, are that aggregated > parsers will not show up in the UI. You cannot create them there. Actually, > any parser not created through the UI (eg CLI) will not show up in the UI, > aggregated or not. > > As a separate issue, I will also volunteer to see if I can help Tamas find > the discuss thread mentioned. It should be linked to the PR or feature > branch for reference. That may also be a gap in dev guidelines that should > be spelled out. > > On Thu, May 2, 2019, 7:17 AM Nick Allen <n...@nickallen.org> wrote: > >> To echo Justin's comments, I am in favor of #2, which provides a clear, >> well-defined path to a release. >> >> - Why hold back a release, especially a point release containing 89 >> improvements, for one issue that will not affect most users? >> >> >> - It is one thing to stall a release to address a bug of limited scope, >> where a fix is well understood and ready for review, but it is >> completely >> another issue to delay for this. >> >> >> - I don't see a set of reviewable PRs yet that will push this over the >> finish line. As has been noted, there were fundamental problems with >> #1360 >> (which has now been closed) that would have prevented adequate review >> by >> the community. >> >> >> - Why drive this issue with the pressure of a stalled release, instead >> of just releasing the fix when it is ready and has been adequately >> reviewed? Swarming on an issue does not often produce quality results. >> >> For those in favor of #1, can someone please provide a clear outline of >> what the fix looks-like? How many PRs will this require? When are these >> PRs likely to be ready? Who is driving this? Tamás has already commented >> that this not a quick fix. This path is very murky to me, but maybe I am >> just ignorant on this. >> >> I would also urge other committers and users who don't have a binding vote >> on the release to share their opinion on the path forward. >> >> >> >> >> On Thu, May 2, 2019 at 7:17 AM Otto Fowler <ottobackwa...@gmail.com> >> wrote: >> >> > If you can find a link in the archives for that thread, it would really >> > help. >> > >> > I don’t think sending them up as one sensor would work…. as something >> > quick. I think it is an interesting idea from a higher level that would >> > need some more thought though ( IE: what if every sensor in the ui was a >> > sensor group, and the existing entries where just groups of 1 ). >> > >> > As far as I can see, we have brought up the idea of a release >> ourselves, I >> > don’t see why we don’t just swarm this issue and get it right then >> release. >> > >> > >> > >> > On May 2, 2019 at 04:16:31, Tamás Fodor (ftamas.m...@gmail.com) wrote: >> > >> > In PR#1360 we introduced a new state management strategy involving a new >> > module called Ngrx. We had a discussion thread on this a few months ago >> and >> > we successfully convinced you about the benefits. This is one of the >> > reasons why this PR is going to be still huge after cleaning up the >> commit >> > history. After you having a look at the changes and the feature itself, >> > there's likely have questions about why certain parts work as they do. >> The >> > thing what I'd like to point out is that, yes, it probably takes more >> time >> > to get it in. >> > >> > In order to being able to release the RC, wouldn't it be an easy and >> quick >> > fix on the backend if it sent the aggregated parsers to the client as >> they >> > were one sensor? It's just an idea, it might be wrong, but at least we >> > shouldn't have to wait until the aforementioned PR gets ready to be >> merged >> > to the master. >> > >> > On Wed, May 1, 2019 at 4:16 PM Justin Leet <justinjl...@gmail.com> >> wrote: >> > >> > > Short version: I'm in favor of #2 of 0.7.1 and #1 as a blocker for >> 0.8.0. >> > > #3 seems like a total waste of time and effort. >> > > >> > > The wall of text version: >> > > I agree this isn't "just the wrong thing shown", but for completely >> > > different reasons. >> > > >> > > To be extremely clear about what the problem is: Our "dev" environment >> > > (whose very name implies the audience is develops) uses a >> > performance-based >> > > advanced feature to ensure that all our of sample flows are regularly >> run >> > > and produce data. This feature has a bare minimal implementation to be >> > > enabled via Ambari, which it currently is by default. This is because >> of >> > > the limited resources available that previously resulted in us turning >> > off >> > > Yaf, and therefore testing it during regular full dev runs. Right now >> > > however, this feature is not exposed through the management UI, and >> > > therefore it isn't obvious what the implications are. Am I missing >> > anything >> > > here? >> > > >> > > For users actually choosing to use the parser aggregation feature in a >> > > non-full-dev environment, I'd expect substantially more care to be >> > involved >> > > given the lack of easy configuration for it (after all, why would you >> > > bother running the aggregated parser alongside the regular parser? >> This >> > > could be more explicitly stated, but again that feels like a doc >> problem. >> > > Right now I could essentially provide two of the same parser and >> create >> > the >> > > same problem, so right now aggregation is only special because it >> runs on >> > > dev by default). This is, in my opinion, primarily a first impression >> > > problem and likely one of many areas that could use improved >> > documentation. >> > > >> > > Quite frankly, I think the issue pointed out here could mostly be >> > resolved >> > > by documenting how the current aggregation is done in dev, and telling >> > how >> > > to change it. Especially for a 0.x.1 release, which is primarily bug >> > > fixes. As can be inferred from my vote, I don't think this problem is >> a >> > > problem that needs solving in a point release. I would support >> improving >> > > the documentation, both full-dev and for aggregation in general for >> the >> > > 0.7.1 point release, while making a 0.8.0 release contingent upon the >> > > outstanding PRs to enable it in the management UI. >> > > >> > > There are a couple deeper issues, imo, that I care substantially more >> > about >> > > than this in particular >> > > * The dev environment is being used as our intro for users, because >> it's >> > > convenient for us to not maintain more environments (which has been a >> > major >> > > pain point in the past). Worse, the dev environment strongly implies >> it's >> > > for Metron developers, rather than people looking to build on top of >> > > Metron. We need an actual strategy for providing end users a clean >> > > impression of Metron (this could be clarifying what the expectations >> of >> > > full dev are, renaming it to something like "full-demo", something >> more >> > > involved, etc.). This is something that we've needed for awhile in >> > general, >> > > and includes larger topics like improving our website, potentially >> > > improving the site book, actually publishing our Javadocs somewhere so >> > > people can develop things easier, publishing out info about Stellar >> > > functions in a better manner, etc. >> > > * The fact that parsers are handled in Ambari at all. It's awful and >> > leads >> > > to situations like this. To the best of my knowledge, once we can do >> > > chaining and aggregation in the Management UI, we should be able to >> > > entirely divorce these two overlapping domains. I'd love to see >> parsers >> > > ripped out of Ambari, then full-dev manages all the setup via REST. At >> > that >> > > point, we can easily tell everyone to just use the management UI. >> > > >> > > On Wed, May 1, 2019 at 7:23 AM Otto Fowler <ottobackwa...@gmail.com> >> > > wrote: >> > > >> > > > I think it would help if the full consequences of having the UI show >> > the >> > > > wrong status where listed. >> > > > >> > > > Someone trying metron, will, by default , see the wrong thing in >> the UI >> > > for >> > > > the ONLY sensors they have that are running and doing data. >> > > > >> > > > What happens when they try to start them to make them work? One, >> two or >> > > > all? >> > > > What happens when he edits them or try to add transformations? One, >> two >> > > or >> > > > all? >> > > > What other things can you do with the sensors in the ui? What >> happens? >> > > > >> > > > Are we recommending aggregation on the list and elsewhere for users? >> > Are >> > > > we recommending something that is going to ensure they get into this >> > > > situation? >> > > > >> > > > I think this is more than ‘just the wrong thing shown’ in the ui. >> > > > >> > > > >> > > > >> > > > >> > > > On April 30, 2019 at 20:48:10, Michael Miklavcic ( >> > > > michael.miklav...@gmail.com) wrote: >> > > > >> > > > The vote for RC1 did not pass and I'd like to kickstart some >> discussion >> > > > about what we should do. >> > > > >> > > > I started taking a look at PR#1360 and it looks like this isn't >> quite >> > as >> > > > close to being able go in as I had originally expected. I want to >> talk >> > > > about options here. It seems to me that we can: >> > > > >> > > > 1. Wait for PR#1360 to go in, but this is likely going to take more >> > time >> > > > than originally anticipated >> > > > 2. Accept the issue in full dev, but add some notes in the developer >> > > > docs about the current feature gap and why sensors aren't showing >> > status >> > > in >> > > > the management UI when aggregation is enabled. >> > > > 3. Find some other workable UI solution. >> > > > 4. Other option? >> > > > >> > > > All things considered, I'm personally leaning towards #2 in the >> > > short-term, >> > > > but I think we should probably talk about this a bit before deciding >> > what >> > > > RC2 should be. >> > > > >> > > > Best, >> > > > Mike >> > > > >> > > >> > >> >
