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

web-apps: Ensure in-flight requests are cancelled #6718

Conversation

kimwnasptd
Copy link
Member

While working on the all-namespaces support, follow up of #6706, I noticed that we currently don't cancel inflight requests.

This means that if a user changes the namespace, and there's delay in the requests, then they will keep seeing objects from the previous namespace. Then once new requests succeed the data will be updated accordingly.

To reproduce this you can set a 2s latency in the DevTools and try to switch namespace.

This PR creates a common Angular Service for polling that:

  1. Handles exponential polling
  2. Uses RxJS observables to handle the requests. This way when we unsubscribe the requests get cancelled

/cc @tasos-ale

@google-cla
Copy link

google-cla bot commented Nov 8, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@kimwnasptd kimwnasptd force-pushed the feature-kimwnasptd-jwa-common-improvements branch from 1f72e79 to 87963c1 Compare November 8, 2022 11:16
Add proxy rules to connect the webpack dev server with the backend
Use the new common code path when installing the common packages.
Edit the angular.json file of all our frontend components to not send
Angular analytics. This way we won't be seeing a y/N input each time we
try to build the frontend, which could also block a CI/CD system.
Create an Angular Service that handles the polling. Specifically the
polling function has as input an observable responsible for fetching the
data. It will return a final observable, which will be using the
fetch-observable, that will only be emitting the final data.

This service will take care of:
1. Checking if the new data is new and reset the polling
2. Cancelling in-flight requests, if someone terminates the subscription
The frontend code of VWA will now be using the new Poller Service which
has a pure RxJS implementation underneath. This will make it simpler to
cancel in-flight requests and also moves the reset logic into the common
code.
The frontend code of JWA will now be using the new Poller Service which
has a pure RxJS implementation underneath. This will make it simpler to
cancel in-flight requests and also moves the reset logic into the common
code.
The frontend code of TWA will now be using the new Poller Service which
has a pure RxJS implementation underneath. This will make it simpler to
cancel in-flight requests and also moves the reset logic into the common
code.
@kimwnasptd kimwnasptd force-pushed the feature-kimwnasptd-jwa-common-improvements branch from 87963c1 to 170b5a3 Compare November 8, 2022 11:18
@tasos-ale
Copy link
Contributor

/lgtm

@elikatsis
Copy link
Member

/approve

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: elikatsis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 7952c8b into kubeflow:master Nov 10, 2022
@kimwnasptd kimwnasptd deleted the feature-kimwnasptd-jwa-common-improvements branch November 10, 2022 14:37
maroroman pushed a commit to maroroman/kubeflow that referenced this pull request Feb 7, 2023
* vwa(front): Use ng serve for local dev

Add proxy rules to connect the webpack dev server with the backend

* twa(front): Use ng serve for local dev

Refs arrikto/dev#1597

* vwa(back): Fix the install-deps make rule

Use the new common code path when installing the common packages.

* wa(front): Disable Angular analytics

Edit the angular.json file of all our frontend components to not send
Angular analytics. This way we won't be seeing a y/N input each time we
try to build the frontend, which could also block a CI/CD system.

* wa(front): Add exponential-backoff service

Create an Angular Service that handles the polling. Specifically the
polling function has as input an observable responsible for fetching the
data. It will return a final observable, which will be using the
fetch-observable, that will only be emitting the final data.

This service will take care of:
1. Checking if the new data is new and reset the polling
2. Cancelling in-flight requests, if someone terminates the subscription

* vwa(front): Use the new Polling Service

The frontend code of VWA will now be using the new Poller Service which
has a pure RxJS implementation underneath. This will make it simpler to
cancel in-flight requests and also moves the reset logic into the common
code.

* jwa(front): Use the new Polling Service

The frontend code of JWA will now be using the new Poller Service which
has a pure RxJS implementation underneath. This will make it simpler to
cancel in-flight requests and also moves the reset logic into the common
code.

* twa(front): Use the new Polling Service

The frontend code of TWA will now be using the new Poller Service which
has a pure RxJS implementation underneath. This will make it simpler to
cancel in-flight requests and also moves the reset logic into the common
code.
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

3 participants