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

Filter incident by tags using simple input field #1114

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

johannaengland
Copy link
Contributor

@johannaengland johannaengland commented Jan 8, 2025

Closes #1044.

This is the simplest implementation without any error handling, since I am working on implementing active search for tags.
The tags need to be entered in the format a=b, c=d.

@johannaengland johannaengland added enhancement New feature or request frontend Affects frontend HTMx Views, urls, templates... labels Jan 8, 2025
@johannaengland johannaengland self-assigned this Jan 8, 2025
Copy link

github-actions bot commented Jan 8, 2025

Test results

   10 files  1 060 suites   38m 16s ⏱️
  536 tests   535 ✅  1 💤 0 ❌
5 360 runs  5 350 ✅ 10 💤 0 ❌

Results for commit dc7b986.

♻️ This comment has been updated with latest results.

@hmpf
Copy link
Contributor

hmpf commented Jan 13, 2025

All tags should be shown to all users. Tags cannot have owners, the fact that that's how we store them is an implementation ERROR. A "wat", to be exact. We have issues about changing how we store tags in the database already: #136, #817.

Copy link
Contributor

@podliashanyk podliashanyk left a comment

Choose a reason for hiding this comment

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

See styling RFC below. Works otherwise

src/argus/htmx/incident/filter.py Outdated Show resolved Hide resolved
@hmpf
Copy link
Contributor

hmpf commented Jan 13, 2025

When I try to run this, I very quickly run out of memory so badly that I either need to kill the test webserver or boot my machine. I suspect it is due to me testing with a copy of production data; our production currently has more than 14000 different, unique tags.

I do not recommend attempting to cram 14000+ tags into a single html file as drop-down options. I suspect type-ahead is an absolute necessity for tags if we are to only accept valid tags.

Also in production we are only using 30 different Tag keys. We could preload just the keys and have ppl select twice: first the key, then the value.

Get the number of values relative to keys by running Tag.objects.values("key").annotate(count=Count("value") in the shell or

SELECT "argus_incident_tag"."key", COUNT("argus_incident_tag"."value") AS "count" 
FROM "argus_incident_tag" GROUP BY "argus_incident_tag"."key"

in the dbshell.

Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

Cannot work as is with production data, too many tags.

@hmpf
Copy link
Contributor

hmpf commented Jan 13, 2025

On my machine the limit seems to be somewhere between 5000 and 7500 tags. 5000 tags eventually loads, 7500 always croaks.

@johannaengland johannaengland force-pushed the filter-incidents-tags-dropdown branch from 045dbf7 to a305a25 Compare January 20, 2025 13:14
@johannaengland johannaengland changed the title Add tag dropdown to filter incidents Filter incident by tags using simple input field Jan 20, 2025
@johannaengland johannaengland force-pushed the filter-incidents-tags-dropdown branch from a305a25 to dc7b986 Compare January 20, 2025 13:18
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 79.62%. Comparing base (bfa8920) to head (dc7b986).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/argus/htmx/incident/filter.py 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1114      +/-   ##
==========================================
- Coverage   79.67%   79.62%   -0.05%     
==========================================
  Files         141      141              
  Lines        5288     5292       +4     
==========================================
+ Hits         4213     4214       +1     
- Misses       1075     1078       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request frontend Affects frontend HTMx Views, urls, templates...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tags-input to "filter incidents"
4 participants