change: [M3-8450] - Lower Events historical data fetching to 7 days #10899
change: [M3-8450] - Lower Events historical data fetching to 7 days #10899abailly-akamai merged 7 commits intolinode:developfrom
Conversation
abailly-akamai
changed the title
abailly-akamai
requested review from
bnussman-akamai,
cpathipa and
jdamore-linode
and removed request for
a team
| cy.wait('@getEventsPoll'); | ||
| cy.get('@getEventsPoll.all').should('have.length', 1); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
@jdamore-linode I would appreciate a second pair of 👀 on both the tests since I had to make some changes to them - thx!
There was a problem hiding this comment.
@jdamore-linode The 2s & 16s polling tests are the ones failing in the CI and i have no idea why. It's passing fine locally, but can't see what CY failure details
There was a problem hiding this comment.
Checking this out now! Kinda have to refresh my memory on how all this works/what the error was/etc. I'm not super confident in these tests to begin seeing as I had to lean on some kinda unusual (and probably hacky) patterns, so I may take a second pass and try to spruce them up while I'm at it.
|
Coverage Report: ✅ |
| }); | ||
|
|
||
| /** | ||
| * - Confirms that Cloud Manager polls the events endpoint 16 times per second. |
There was a problem hiding this comment.
| * - Confirms that Cloud Manager polls the events endpoint 16 times per second. | |
| * - Confirms that Cloud Manager polls the events endpoint every 16 seconds. |
| }); | ||
|
|
||
| /** | ||
| * - Confirms that Cloud Manager polls the events endpoint 2 times per second when there are in-progress events. |
There was a problem hiding this comment.
| * - Confirms that Cloud Manager polls the events endpoint 2 times per second when there are in-progress events. | |
| * - Confirms that Cloud Manager polls the events endpoint every 2 seconds when there are in-progress events. |
packages/manager/cypress/e2e/core/notificationsAndEvents/events-fetching.spec.ts
Outdated
Show resolved
Hide resolved
|
@jdamore-linode is it worth adding an assertion to verify that the in-progress event is actually included in the response, to ensure that the increased polling rate is indeed triggered by the presence of an in-progress event? May not be necessary, but wanted to check |
@jaalah-akamai If these were end-to-end tests using real events we'd definitely do that, but we're already implicitly doing this since we're defining the mocked responses, and setting them up not to contain an in-progress event in one test and to contain an in-progress event in the other (which, iirc, is basically the only real difference between the two tests -- I think I literally copy/pasted them when I wrote them since we were in such a rush at the time, but in reality it might make sense to parameterize them or something since that might kind of highlight at a glance what the difference between the two flows is) |
|
@jdamore-linode How do you feel about moving forward with either skipping the test for now until we have a better handle on it. We now have very few days for this to sit in dev and the only test issue seems irrelevant to the purpose of this PR |
3 participants
This PR is a replacement for #10785 which had become difficult to rebase.
It is essentially the same PR including fixes and cleanup for the new e2e tests.
Description 📝
This change optimizes Cloud Manager's event querying by reducing the historical data range.
Specifically, it limits the event query timeframe from the current 90 days to a more focused 7-day period.
/eventspage still queries 90 day and isn't affected by this PRThis adjustment aims to improve query performance and reduce unnecessary data retrieval while still providing relevant recent event information.
Changes 🔄
Target release date 🗓️
Next Release: 9/16/2024
How to test 🧪
Note
We are working to have this fully covered via E2E tests in an upcoming PR cc: @jdamore-linode
Prerequisites
Verification steps
As an Author I have considered 🤔
Check all that apply