lix-community/nix-security-tracker
0
0
Fork 0
Code Issues 94 Pull requests 9 Projects Releases Packages Wiki Activity

fix: refresh suggestion cache entry on derivations update #384

Merged
erictapen merged 5 commits from fix-cache into main 2024-12-02 13:34:53 +00:00
Conversation 12 Commits 5 Files changed 4 +34 -11
erictapen commented 2024-11-27 01:04:06 +00:00 (Migrated from github.com)
Copy link

Couldn't sleep with having this problem in my head.

I had to switch the arguments for the update_or_create function because they were wrong: The proposal_id is used for the lookup and the payload is then used on update.

This is going to make updates very slow, but doesn't affect page load and I'd argue that it is important to have correct behaviour at first. With https://github.com/Nix-Security-WG/nix-security-tracker/pull/383 merged update times wouldn't even be noticable.

Couldn't sleep with having this problem in my head. I had to switch the arguments for the update_or_create function because they were wrong: The proposal_id is used for the lookup and the payload is then used on update. This is going to make updates very slow, but doesn't affect page load and I'd argue that it is important to have correct behaviour at first. With https://github.com/Nix-Security-WG/nix-security-tracker/pull/383 merged update times wouldn't even be noticable.
RaitoBezarius (Migrated from github.com) reviewed 2024-11-27 01:04:06 +00:00
fricklerhandwerk (Migrated from github.com) reviewed 2024-11-27 08:19:21 +00:00
src/website/webview/views.py
fricklerhandwerk (Migrated from github.com) commented 2024-11-27 08:19:21 +00:00
Copy link

This should be done triggered async, otherwise we'll have to wait a long time. Right now it's also done every time, even if pkgs are not updated.

This should be done triggered async, otherwise we'll have to wait a long time. Right now it's also done *every* time, even if pkgs are not updated.
RaitoBezarius commented 2024-11-28 15:46:30 +00:00 (Migrated from github.com)
Copy link

I looked into that more seriously, I don't think triggering a recaching is the good approach here.

For one, this will trigger a weird experience where you will see temporarily (in case of heavy loads) the view non-sync with what's going on.

Recaching should be reserved for expensive transformations that are hard to re-express on their cached variants.

In this instance, there's only one operation: filtering out derivations from the payload.

This should be expressible in one SQL query which should not involve back'n'forth with Python. I have some local prototype I'm working on where I can do this sort of stuff.

I looked into that more seriously, I don't think triggering a recaching is the good approach here. For one, this will trigger a weird experience where you will see temporarily (in case of heavy loads) the view non-sync with what's going on. Recaching should be reserved for expensive transformations that are hard to re-express on their cached variants. In this instance, there's only one operation: filtering out derivations from the payload. This should be expressible in one SQL query which should not involve back'n'forth with Python. I have some local prototype I'm working on where I can do this sort of stuff.
👍 1 👀 1
fricklerhandwerk commented 2024-11-28 15:57:34 +00:00 (Migrated from github.com)
Copy link

Are you suggesting something like storing a flag for the filtered-out derivations, but somewhere outside the actual suggestion entry (since that is cached)?

Are you suggesting something like storing a flag for the filtered-out derivations, but somewhere outside the actual suggestion entry (since that is cached)?
RaitoBezarius commented 2024-11-29 17:50:34 +00:00 (Migrated from github.com)
Copy link

This is now ready.

This is now ready.
RaitoBezarius commented 2024-11-29 17:52:19 +00:00 (Migrated from github.com)
Copy link

Rebased.

Rebased.
RaitoBezarius commented 2024-11-29 17:55:07 +00:00 (Migrated from github.com)
Copy link

The rebase broke displaying derivations.

The rebase broke displaying derivations.
RaitoBezarius commented 2024-11-29 18:00:50 +00:00 (Migrated from github.com)
Copy link

OK this is beyond my paygrade, I do see the derivations threaded in some templates but I don't understand what's going on, can you weigh in @erictapen (whenever you are available ofc)?

OK this is beyond my paygrade, I do see the derivations threaded in some templates but I don't understand what's going on, can you weigh in @erictapen (whenever you are available ofc)?
RaitoBezarius commented 2024-11-29 18:01:03 +00:00 (Migrated from github.com)
Copy link

In the meantime, fix #396 as intended in principle.

In the meantime, fix #396 as intended in principle.
fricklerhandwerk commented 2024-11-30 15:18:01 +00:00 (Migrated from github.com)
Copy link

This also breaks state change display, setting it to draft for now.

This also breaks state change display, setting it to draft for now.
erictapen commented 2024-12-02 12:43:57 +00:00 (Migrated from github.com)
Copy link

Fixed!

Fixed!
fricklerhandwerk (Migrated from github.com) approved these changes 2024-12-02 13:34:11 +00:00
fricklerhandwerk (Migrated from github.com) left a comment
Copy link

Tested, works. Displaying pages in the "selected" view for some reason is much slower than in the "suggestions" view, which doesn't make much sense to me, but we'll deal with that another time.

Tested, works. Displaying pages in the "selected" view for some reason is much slower than in the "suggestions" view, which doesn't make much sense to me, but we'll deal with that another time.
Sign in to join this conversation.
Reviewers
No reviewers
RaitoBezarius
fricklerhandwerk
Labels
Clear labels   
automation

   
backend

something the web server needs to provide the frontend with

  
bug

Something isn't working

  
contributor experience

   
data

something about quality or quantity of ingested data

  
deployment

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
nice to have

Nice to have, but not necessary for the time being

  
notifications

   
package maintainer

user story for package maintainers

  
performance

   
skin

Anything related to the visual presentation

  
tech debt

Activities that reduce tech debt

  
user story

description or implementation of a workflow

No labels
automation
backend
bug
contributor experience
data
deployment
documentation
duplicate
good first issue
help wanted
nice to have
notifications
package maintainer
performance
skin
tech debt
user story
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lix-community/nix-security-tracker#384
No description provided.
Delete branch "fix-cache"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?