BC-7561 - separate job for failed batch deletion requests #5448
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this commit changes failed deletion and counted pending requests to also be older than configured threshold
add new config var for when pending or failed should be no longer processed
virgilchiriac
force-pushed
the
BC-7561-batch-delete-fix
branch
from
979f793 to
ef53fad
Compare
virgilchiriac
force-pushed
the
BC-7561-batch-delete-fix
branch
from
a6bef9f to
ee088b5
Compare
virgilchiriac
force-pushed
the
BC-7561-batch-delete-fix
branch
2 times, most recently
from
ceaa2f4 to
b9b6c83
Compare
virgilchiriac
force-pushed
the
BC-7561-batch-delete-fix
branch
from
b9b6c83 to
8b8e624
Compare
1 task
virgilchiriac
force-pushed
the
BC-7561-batch-delete-fix
branch
from
165f606 to
51ea68e
Compare
virgilchiriac
changed the title
apps/server/src/modules/deletion-console/deletion-client/deletion.client.ts
Outdated
Show resolved
Hide resolved
apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts
Outdated
Show resolved
Hide resolved
| ], | ||
| }, | ||
| ], | ||
| const newerThan = new Date(Date.now() - 15 * 60 * 1000); // 15 minutes ago |
There was a problem hiding this comment.
setting variable instead of comment? (same for below)
| @@ -851,6 +851,8 @@ describe('UserService', () => { | |||
| userRepo.findByIdOrNull.mockResolvedValueOnce(user); | |||
| userRepo.deleteUser.mockResolvedValue(1); | |||
|
|
|||
| config.get.mockReturnValue(true); | |||
There was a problem hiding this comment.
maybe specify which config get should be mocked instead of a general mock return?
EzzatOmar
reviewed
Jan 23, 2025
apps/server/src/modules/deletion-console/builder/trigger-deletion-execution-options.builder.ts
Outdated
Show resolved
Hide resolved
| @@ -17,5 +16,5 @@ export class DeletionRequestBodyProps { | |||
| required: true, | |||
| nullable: false, | |||
| }) | |||
| deleteInMinutes?: number = MINUTES_OF_30_DAYS; | |||
| deleteAfterMinutes?: number; | |||
There was a problem hiding this comment.
it is unconventional to use minutes instead of seconds as base time unit
There was a problem hiding this comment.
true, but also makes overwrites if needed, harder to calculate. Had the same thought to stick to milliseconds, but is not practical either
I just kept the it, just rename the var to better describe what it actually does and relate to db field
virgilchiriac
changed the title
uidp
reviewed
Jan 24, 2025
...rver/src/modules/deletion-console/builder/trigger-deletion-execution-options.builder.spec.ts
Outdated
Show resolved
Hide resolved
apps/server/src/modules/deletion/repo/scope/deletion-request-scope.ts
Outdated
Show resolved
Hide resolved
virgilchiriac
force-pushed
the
BC-7561-batch-delete-fix
branch
2 times, most recently
from
2b16c4f to
c0df280
Compare
virgilchiriac
force-pushed
the
BC-7561-batch-delete-fix
branch
from
c0df280 to
e216fc9
Compare
|
uidp
reviewed
Jan 27, 2025
| @@ -26,9 +26,9 @@ export class DeletionClient { | |||
| } | |||
| } | |||
|
|
|||
| public async executeDeletions(limit?: number): Promise<void> { | |||
| public async executeDeletions(limit?: number, runFailed?: boolean): Promise<void> { | |||
There was a problem hiding this comment.
Would it make sense to use an options object type instead of sereate parameters? Would better show the independence of limit and runFailed IMHO. (maybe also a matter of taste)
uidp
approved these changes
Jan 27, 2025
avoid loading node by checking how many requests are already currently in progress
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Links to Tickets or other pull requests
BC-7561
hpi-schul-cloud/dof_app_deploy#1095
Approval for review
generate-client:serverwas executed in vue frontend and changes were tested and put in a PR with the same branch name.