Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
This repository was archived by the owner on Jan 24, 2024. It is now read-only.

SEP 17 - Job acknowledgement events#23

Merged
waynew merged 1 commit intosaltstack:masterfromlukasraska:job-ack-event
Jan 6, 2020

Conversation

@lukasraska
Copy link
Contributor

Rendered markdown:https://github.com/lukasraska/salt-enhancement-proposals/blob/job-ack-event/job-ack-event.md

Basic idea is to be able to determine whether minion received job event, or not (due to various reasons). Particularly usable in integrated systems to determine if retrying is an option.

@lukasraskalukasraska requested a review froma team as acode ownerOctober 24, 2019 12:45
@ghost ghost requested review fromdwoz and removed request fora teamOctober 24, 2019 12:45
@thatch45
Copy link
Contributor

I LOVE this SEP, I made the saltutil.running system before beacons and the event bus was in full swing! I would also consider leveraging a beacon that also just fired what jobs are running on a regular cadence which would be much smoother than how we track running jobs now

waynew reacted with thumbs up emoji

@lukasraska
Copy link
ContributorAuthor

Beacon could be an option here, to periodically send all running jobs. That will however not help 100% here. It will almost certainly miss short-running jobs (but then you will receive return event from the job, so not that big of an issue), but it won't be "realtime" and will always fire.

If the beacon is set up for every minute and the job lasts 30 minutes, you get 30 events instead of single ack. But as temporary solution it's usable. I was also looking into how engines are working on minion, but it seems it can only listen for fired events from minion, not the opposite way (so essentially it works as on master, you can catch what minion sends, not what master sends).

@waynew
Copy link
Contributor

Yeah, I don't have much to add other than this is a fantastic idea :)

@waynewwaynew changed the titleJob acknowledgement eventsSEP 17: Job acknowledgement eventsNov 14, 2019
@waynewwaynew changed the titleSEP 17: Job acknowledgement eventsSEP 17 - Job acknowledgement eventsNov 14, 2019
@lukasraska
Copy link
ContributorAuthor

@waynew Could we get this moved to "Final Comment Period"? :)

@waynewwaynew added the Final Comment PeriodSpeak now or forever hold your peace. labelDec 5, 2019
@waynew
Copy link
Contributor

@lukasraska yeah, sorry for the delay - there has been much travel and holiday time. I'll bring it up in our core meeting tomorrow.

lukasraska reacted with thumbs up emoji

@waynew
Copy link
Contributor

@lukasraska I brought it up in our meeting, and the consensus is 👍

There should be some official votes coming in soon and we can get this approved/merged. In our discussion it came up that this is an awesome idea, but it's potentially a large undertaking.@thatch45 has a bunch of information that he plans to share, but this week is super busy for him. You should be able to expect a response from him next Friday.

Let us know if you have any other questions!

@lukasraska
Copy link
ContributorAuthor

Great, I have PR for this ready (with tests, more or less), so I will just wait for follow-up

@lukasraska
Copy link
ContributorAuthor

Any comment to this SEP,@thatch45 ?
If not, I will prepare the implementation and it can be incrementally enhanced as the details arise. Thanks

waynew added a commit that referenced this pull requestJan 6, 2020
@waynewwaynew merged commit01da6bc intosaltstack:masterJan 6, 2020
@waynewwaynew added Accepted and removed Final Comment PeriodSpeak now or forever hold your peace. labelsJan 6, 2020
@waynew
Copy link
Contributor

@lukasraska This SEP has been accepted 👍

Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@dwozdwozAwaiting requested review from dwoz

5 more reviewers

@waynewwaynewwaynew approved these changes

@s0undt3chs0undt3chs0undt3ch approved these changes

@Ch3LLCh3LLCh3LL approved these changes

@Akm0dAkm0dAkm0d approved these changes

@cmcmarrowcmcmarrowcmcmarrow approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@lukasraska@thatch45@waynew@s0undt3ch@Ch3LL@Akm0d@cmcmarrow

[8]ページ先頭

©2009-2025 Movatter.jp