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

Fix scheduled task purge failing without user #5896

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

mdellweg
Copy link
Member

When a task was dispatched by a task schedule, there is no user attached to it. So purge tasks should just look at all tasks. However when a user gets deleted after dispatching that task, the task will run with current_user=None also. We need to make sure not to purge too many tasks in this case.

fixes #5881

@mdellweg mdellweg marked this pull request as ready for review October 11, 2024 08:19
@@ -0,0 +1 @@
Fixed task purge to not expect a user when run scheduled.
Copy link
Member

Choose a reason for hiding this comment

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

s/run scheduled/run is scheduled

Copy link
Member Author

@mdellweg mdellweg Oct 11, 2024

Choose a reason for hiding this comment

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

Is that the difference between adjective and adverb?
Anyway both of us not being native english speakers, we should try to use more simple language. What about:
"... when running from a schedule."?

Copy link
Contributor

Choose a reason for hiding this comment

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

"scheduled run" is an adjective. "run is scheduled" is a verb formation. "run scheduled" is...an adverb, because it's being used to describe how a ting was/is run? But it's an awkward construct, to my native-speaking ear. This is, of course, English, where awkward constructs are understandable.

Perhaps "...to not expect a user, when a run has been scheduled by Pulp itself." Wordy, but makes everything explicit?

Copy link
Member Author

Choose a reason for hiding this comment

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

Explicit enough for everybody is what i want to aim for.

@mdellweg
Copy link
Member Author

After talking to @ggainey i think there might be a better solution than the weird. Heuristic applied here. Marking as draft again.

@mdellweg mdellweg marked this pull request as draft October 11, 2024 13:03
When a task was dispatched by a task schedule, there is no user attached
to it. Such purge tasks should just look at all tasks.

fixes pulp#5881
@mdellweg mdellweg marked this pull request as ready for review October 11, 2024 13:38
current_user = get_current_authenticated_user()
assert current_user is not None, (
"This task should have been dispatched by a user. Cannot find it though. "
"Maybe it got deleted."
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -0,0 +1 @@
Fixed task purge to not expect a user, when a run has been scheduled by Pulp itself.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@mdellweg mdellweg merged commit 2bef8a3 into pulp:main Oct 11, 2024
12 checks passed
@mdellweg mdellweg deleted the 5881_purge_scheduled branch October 11, 2024 21:29
Copy link

patchback bot commented Oct 11, 2024

Backport to 3.65: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.65/2bef8a395ee9fea20a5ff1a6903b73f959187cf4/pr-5896

Backported as #5899

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

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