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

Support sorting by last QA started time #1712

Merged
merged 5 commits into from Apr 22, 2024
Merged

Support sorting by last QA started time #1712

merged 5 commits into from Apr 22, 2024

Conversation

ikreymer
Copy link
Member

To support #1683, it would be useful to be able to sort by 'last QA start time' in addition to/instead of last QA state.

backend/btrixcloud/basecrawls.py Outdated Show resolved Hide resolved
backend/btrixcloud/crawls.py Outdated Show resolved Hide resolved
backend/test/test_qa.py Show resolved Hide resolved
Include active QA in these (will already become first since we're
sorted by started) instead of separating it out into a separate
field. This is more consistent with how we handle workflows.

We do however keep activeQAStats, as that can be used to quickly
determine if there is an active QA run and show a progress bar for
it if so.
@tw4l
Copy link
Contributor

tw4l commented Apr 21, 2024

@ikreymer I took a crack at simplifying the QA sorting fields and making them more consistent with what we do for workflows. Namely:

  • sortBy fields renamed to lastQAState and lastQAStarted
  • Current QA runs are now included in the lastQAState/lastQAStarted fields, rather than being separated out to different values
  • Kept activeQAStats as-is for progress bar.

Ready for your eyes.

This will require some small tweaks to #1683 , will push those next. And if we want to revert the changes, can always just drop the commit.

tw4l added a commit that referenced this pull request Apr 21, 2024
- Rename QA sort fields to `lastQAState` and `lastQAStarted`
- Include active/current QA in last, to be consistent with workflows
- Check if current QA based on `activeQAStats`, since we now removed
`activeQAState` as redundant
@tw4l
Copy link
Contributor

tw4l commented Apr 21, 2024

#1683 updated to match!

SuaYoo pushed a commit that referenced this pull request Apr 22, 2024
SuaYoo pushed a commit that referenced this pull request Apr 22, 2024
- Rename QA sort fields to `lastQAState` and `lastQAStarted`
- Include active/current QA in last, to be consistent with workflows
- Check if current QA based on `activeQAStats`, since we now removed
`activeQAState` as redundant
@ikreymer
Copy link
Member Author

Thanks for the update! Only question is if new operators like concatArrays would effect perf, but since this is all on one object, hopefully fairly negligible.

@ikreymer ikreymer merged commit 1844e76 into main Apr 22, 2024
4 of 5 checks passed
@ikreymer ikreymer deleted the sort-last-qa-started branch April 22, 2024 20:00
@tw4l
Copy link
Contributor

tw4l commented Apr 22, 2024

Thanks for the update! Only question is if new operators like concatArrays would effect perf, but since this is all on one object, hopefully fairly negligible.

FWIW they definitely do to some degree but we can see if that becomes a problem. Likely the most performant solution longterm is to precompute more of these values, but that comes with its own complexity tradeoff so I'd wait to see if it becomes an issue.

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

2 participants