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

feat(match-expression): match expression should be evaluated on backend #1069

Merged
merged 14 commits into from
Aug 1, 2023

Conversation

tthvo
Copy link
Member

@tthvo tthvo commented Jul 27, 2023

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed the last commit: git commit --amend --signoff

Fixes: #1067
Depends on https://github.com/cryostatio/cryostat/issues/1601

Description of the change:

Update front-end handler for async checks when evaluating match expression (i.e. done on server via endpoint /api/beta/matchExpression)

Motivation for the change:

See #1067

@tthvo tthvo added the feat New feature or request label Jul 27, 2023
@mergify mergify bot added the safe-to-test label Jul 27, 2023
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-18b0558a5ba8e57f52f23454bddb018e17254e9d sh smoketest.sh

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-225a55fa5ee0f72f5cc414e526498fdda2508b39 sh smoketest.sh

@tthvo
Copy link
Member Author

tthvo commented Jul 27, 2023

Remaining tasks:

  • Use switchMap to cancel/ignore previous API request. Seems like using the expression hook might not be needed anymore. Just use the expression service directly.
  • Fix unit tests. Add more tests.
  • Update API request (/beta/matchExpression) body to include JSON instead of multi-part. Depends on https://github.com/cryostatio/cryostat/issues/1601

@github-actions
Copy link

This PR/issue depends on:

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-c5ee68df710d9525247cd017f36cff5708353b5b sh smoketest.sh

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-c366a5dd55f1a5ad8d2ec4af4625a1846bb9877c sh smoketest.sh

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-32d2e819dd60b5f7598a1720ec989f946e9ff93c sh smoketest.sh

@tthvo tthvo requested review from maxcao13 and aali309 August 1, 2023 08:37
@tthvo tthvo marked this pull request as ready for review August 1, 2023 08:39
@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-351bcd026fb4a30baba45ea1ede0cc101a189e9f sh smoketest.sh

@tthvo
Copy link
Member Author

tthvo commented Aug 1, 2023

Ready for review now!

There are a lot more changes than expected but I tried my best to minimize the number of API requests to /matchExpressions endpoint.

One fix that might raise questions is in the stored credential table. When a target appears or disappears, I opted to just refresh the entire table (1 API request) instead of each match expression checking themselves (a lot API requests and React reducer is not fit for async handling).

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-1e805a1c05bca0cb49e813464891442e9b52d89d sh smoketest.sh

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-00afc5646575d4b6c35ea5ec04b37a4b8f7ac9c6 sh smoketest.sh

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-dc409adbaf93a8c532129a27c7a1243423e2a194 sh smoketest.sh

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-624b32addf1921f6fc920f9d1d12990faceb5495 sh smoketest.sh

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1069-edebacc8bebe1007807023edfb518bba1ac276b3 sh smoketest.sh

@andrewazores andrewazores merged commit 002506c into cryostatio:main Aug 1, 2023
@tthvo tthvo deleted the match-exp branch August 1, 2023 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

[Task] Match expression evaluation should be done by server
2 participants