metalk8s
metalk8s copied to clipboard
UI: improve "refresh" sagas
Component: UI
Why this is needed:
- In the UI, to refresh resources in a page we call "refresh" sagas that look like:
export function* refreshAlerts() {
yield put(updateAlertsAction({ isRefreshing: true }));
const resultAlerts = yield call(fetchAlerts);
if (!resultAlerts.error) {
yield delay(REFRESH_TIMEOUT);
const isRefreshing = yield select(
state => state.app.monitoring.alert.isRefreshing,
);
if (isRefreshing) {
yield call(refreshAlerts);
}
}
}
and to stop the refresh, we call "stopRefresh" sagas that look like:
yield put(updateAlertsAction({ isRefreshing: false }));
This "refresh" management is split in 2 different sagas, so it is hard to understand. Moreover, the "refresh" saga has to wait REFRESH_TIMEOUT seconds (since delay is a blocked call) to know if it has to relaunch the refresh based on a redux state isRefreshing.
Suggested improvements (cf @gdemonet ):
export function* refreshAlerts() {
while (true) {
yield take(REFRESH_ALERTS);
while (true) {
yield fork(fetchAlerts);
const {interrupt, requeue} = yield race({
interrupt: take(STOP_REFRESH_ALERTS),
requeue: delay(DELAY_TIMEOUT),
});
if (interrupt) { break; }
}
}
}
- if there is an error during
fetchAlerts, we can justyield put({type: STOP_REFRESH_ALERTS}), to stop the refresh in thefetchAlertssaga. - It would be even better to cancel "fetchAlerts" when "interrupt" occurs.
- Consequently,
state.app.monitoring.alert.isRefreshingis only used to display refresh icon.
What should be done:
Implementation proposal (strongly recommended):
Test plan:
Is this still relevant with React Query @JBWatenbergScality?
Not really we should better have a debt ticket about switching the current implementation to react-query 🤔