-
Notifications
You must be signed in to change notification settings - Fork 45
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
implement status signaling #120
implement status signaling #120
Conversation
This change set adds the ability for karton instances to signal to downstream consumers about the beginning and ending of task execution. Signaling is performed by sending corresponding "status" tasks. Task signaling is enabled using the following entry in the karton configuration file: ``` [signaling] status = 1 ``` Task signaling is disabled by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great 👍 Nice use of hooks.
I have only few comments according to the naming:
type:status
=>type:karton.status
ortype:karton.signaling
? It would be nice to provide some separate "namespace" for internal tasks.begin
/end
=>task_begin
/task_end
to make some space for further signals
@psrok1 Good input regarding the proposed name changes. Let me update things accordingly. |
* use `ConfigParser.getboolean` * rename signaling status task identifiers
1f05e4d
to
8efa746
Compare
@@ -39,7 +39,7 @@ def __init__(self, config): | |||
endpoint=config["minio"]["address"], | |||
access_key=config["minio"]["access_key"], | |||
secret_key=config["minio"]["secret_key"], | |||
secure=bool(int(config["minio"].get("secure", True))), | |||
secure=config.config.getboolean("minio", "secure", fallback=True), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to push again, since in the previous change set it read config.getboolean
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, config.config
is quite cumbersome. I think it should be simplified it in the future.
Tested manually, seems to be working! Thanks! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok 👍
Thanks!
This change set adds the ability for karton instances to signal to downstream
consumers about the beginning and ending of task execution. Signaling is
performed by sending corresponding "status" tasks.
Task signaling is enabled using the following entry in the karton
configuration file:
Task signaling is disabled by default.