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

refactor: move project filtering to server side #8102

Merged
merged 1 commit into from Jan 6, 2022

Conversation

alexmt
Copy link
Collaborator

@alexmt alexmt commented Jan 5, 2022

Signed-off-by: Alexander Matyushentsev AMatyushentsev@gmail.com

PR moves improve applications list page performance by moving project filtering to server-side. When a user adds the project filter then both list and stream API will reduce only project-related applications which significantly reduce browser CPU usage. The disadvantage is that use will see a loading indicator after changing project filter:

Screen.Recording.2022-01-05.at.2.37.30.PM.mov

I believe the loading indicator is acceptable in this case because typically end-users select the required project once and don't change it very frequently.

@alexmt alexmt requested review from ciiay, pasha-codefresh, rbreeze, reginapizza and saumeya and removed request for pasha-codefresh January 5, 2022 22:42
@alexmt
Copy link
Collaborator Author

alexmt commented Jan 5, 2022

@ciiay, @rbreeze, @reginapizza, @saumeya, Let me know what you think, please!

@rbreeze
Copy link
Member

rbreeze commented Jan 6, 2022

I agree, I think the loading indicator (which is barely noticeable) is a worthwhile tradeoff.

Copy link
Member

@rbreeze rbreeze left a comment

Choose a reason for hiding this comment

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

LGTM!

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@codecov
Copy link

codecov bot commented Jan 6, 2022

Codecov Report

Merging #8102 (8cf2ee8) into master (722fe92) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8102      +/-   ##
==========================================
- Coverage   41.53%   41.52%   -0.01%     
==========================================
  Files         174      174              
  Lines       22703    22707       +4     
==========================================
  Hits         9430     9430              
- Misses      11932    11936       +4     
  Partials     1341     1341              
Impacted Files Coverage Δ
server/application/application.go 32.65% <0.00%> (-0.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 722fe92...8cf2ee8. Read the comment docs.

@alexmt alexmt merged commit 37851b1 into argoproj:master Jan 6, 2022
@alexmt alexmt deleted the project-filter-server-side branch January 6, 2022 17:39
@saumeya
Copy link
Contributor

saumeya commented Jan 10, 2022

Looks great @alexmt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants