Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements a pattern checker directive for use in the Add Data UI #6493

Merged
merged 6 commits into from
Mar 22, 2016

Conversation

Bargs
Copy link
Contributor

@Bargs Bargs commented Mar 10, 2016

Related to #5974

Provides a re-useable directive for checking the document count of a given index pattern. Intended for use on the final step of add data wizards so that the user can verify that their filebeat/topbeat/foobeat install is working.

screen shot 2016-03-09 at 7 55 25 pm

};

this.validateInstall = () => {
es.count({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to eventually get the es client out of the frontend.
Can we make this a REST endpoint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rashidkpc I asked myself the same question, I didn't know if it made sense for something so simple but if we really want to eliminate the ES client completely then I guess it does.

Any opinions on what the endpoint for such an API should be? I could see any of the following being viable options:

localhost:5601/api/kibana/{indexPatternId}/_count
(mirrors ES syntax)

localhost:5601/api/kibana/ingest/{indexPatternId}/_count
(with the ingest API we've sort of established the ingest config as a higher level concept than an index pattern, so maybe it belongs under ingest?)

localhost:5601/api/kibana/index_patterns/{indexPatternId}/_count
(this one feels weird but I include it because it's a way to namespace the API without re-using ingest)

Something else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the first one, i don't think it belongs under ingest

@rashidkpc
Copy link
Contributor

I'd get rid of both the "Validating" and "Done" buttons here and make them one big disabled button with the text "Waiting for data ..." and just go ahead and start the looper immediately. Then when you see data change the button to green with a big "Success!" and the text "click to above to continue". Or something like that.

Under the big "Waiting for data" button you can stick a small link that says "Skip this step" for folks who don't need your help watching for documents.

@Bargs
Copy link
Contributor Author

Bargs commented Mar 14, 2016

Once #6534 is merged I'll rebase this and then it'll be ready for another look.

@Bargs
Copy link
Contributor Author

Bargs commented Mar 16, 2016

I've addressed all the feedback, but #6553 needs to be merged before this branch can be fully tested.

Instead of creating a disabled button that says "waiting for data", I just removed the button entirely and added an extra step to the instructions explaining that we're polling the index pattern and that they can skip the step by simply clicking the existing "Done" button if they want to.

@Bargs Bargs force-pushed the ingest/filebeatVerify branch from 3127348 to 7cc4945 Compare March 18, 2016 20:53
@Bargs
Copy link
Contributor Author

Bargs commented Mar 18, 2016

@rashidkpc ready for another look.

@rashidkpc rashidkpc assigned rashidkpc and unassigned Bargs Mar 18, 2016
@rashidkpc
Copy link
Contributor

Needs tests, but otherwise GTG

@rashidkpc rashidkpc assigned Bargs and unassigned rashidkpc Mar 18, 2016
@Bargs Bargs force-pushed the ingest/filebeatVerify branch from 838284b to 0893115 Compare March 21, 2016 19:32
@Bargs
Copy link
Contributor Author

Bargs commented Mar 22, 2016

jenkins, test it

@Bargs Bargs force-pushed the ingest/filebeatVerify branch from 0893115 to 615560f Compare March 22, 2016 17:28
@Bargs
Copy link
Contributor Author

Bargs commented Mar 22, 2016

Tests added and build is finally green after merging master a few times, ready for one more look @rashidkpc

@Bargs Bargs assigned rashidkpc and unassigned Bargs Mar 22, 2016
@rashidkpc
Copy link
Contributor

LGTM

rashidkpc pushed a commit that referenced this pull request Mar 22, 2016
Implements a pattern checker directive for use in the Add Data UI
@rashidkpc rashidkpc merged commit e39fc50 into elastic:feature/ingest Mar 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants