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

[BeatsCM] Paginate beats and tags on the API #32667

Conversation

mattapperson
Copy link
Contributor

@mattapperson mattapperson commented Mar 7, 2019

Currently CM paginates beats and tags but it does so in-memory in the browser. Each API request returns ALL beats and tags.

This PR paginates these items on an API level for drastically improved perf on large scale deployments.

Note: Depends on
#31660

Also fixes:

And adds a new feature of:

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

…testing_framework_adapter.ts

Co-Authored-By: mattapperson <me@mattapperson.com>
@elasticmachine
Copy link
Contributor

💔 Build Failed

@mattapperson
Copy link
Contributor Author

mattapperson commented Mar 15, 2019

@justinkambic for the access issue, try clearing browser cache and optimize folder. This was an issue before I made a change to rendering. That change might not be making it through an old optimizer build

@elasticmachine
Copy link
Contributor

💔 Build Failed

@mattapperson mattapperson marked this pull request as ready for review March 15, 2019 14:33
@justinkambic
Copy link
Contributor

justinkambic commented Mar 15, 2019

EDIT: I accidentally conducted this test on master, but the things I pointed out happened the same way on my PR branch, so they are not related to this PR and can probably be disregarded.

So, everything I tried when doing a functional review worked ok.

My process was to:

  • navigate through start-up experience
  • enroll several beats
  • create several tags
  • associate my tags with a few beats
  • associate/disassociate across multiple beats

Something worth mentioning - buttons required double-clicks in some cases (I know this has been an issue in the past, so it may be the same one), and the beats list didn't update after I enrolled beats, only if I navigated to a different page/view and back to it, seen below.

Note

  • after watching this I didn't leave enough frames at the end, there were 4 beats, after enrollment 5 are displayed when I navigate back)
  • you can see me needing to double-click the Done button

Mar-15-2019 12-40-11

Aside from that this looks like it's working ok!

@justinkambic
Copy link
Contributor

Something else I've noticed, I can't seem to disassociate tags from beats using the dropdown on the beats list:
Mar-15-2019 13-21-17

This should take the tag off of the beat right?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@justinkambic
Copy link
Contributor

Ok, I went back and checked the two functional issues I had and they are fixed now. I was able to update the license of my snapshot to platinum from the management tab, and I could remove tags from beats again.

Mar-15-2019 18-56-51

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ph
Copy link
Contributor

ph commented Mar 21, 2019

  1. Should I see some pagination navigation displayed when I change the number of items?

  2. I've tested it locally, I have created 12 tags, I set the row per page to be 10, the list is adjusted to 10, but the rows per page still display the default of 25.

Screen Shot 2019-03-21 at 3 15 14 PM

Screen Shot 2019-03-21 at 3 15 39 PM

@elasticmachine
Copy link
Contributor

Pinging @elastic/beats

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ph
Copy link
Contributor

ph commented Apr 1, 2019

LGTM, need rebase.

@ph
Copy link
Contributor

ph commented May 6, 2019

Let's rebase and merge this @mattapperson

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@jbagel2
Copy link

jbagel2 commented Sep 12, 2019

Is this feature stalled? Or still planed for merge?

@elasticmachine
Copy link
Contributor

💔 Build Failed

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.

None yet

5 participants