Skip to content

#102 Support new task restart semantics#362

Merged
gunnarmorling merged 1 commit into
kcctl:mainfrom
C0urante:gh-102
Jul 31, 2023
Merged

#102 Support new task restart semantics#362
gunnarmorling merged 1 commit into
kcctl:mainfrom
C0urante:gh-102

Conversation

@C0urante

Copy link
Copy Markdown
Contributor

Fixes #102

Usage:

Usage: kcctl restart connector [-e] [-t=<tasks>] [NAME...]
Restarts the specified connector(s)
      [NAME...]         Name of the connector (e.g. 'my-connector')
  -e, --reg-exp         use NAME(s) as regexp pattern(s) to use on all
                          connectors
  -t, --tasks=<tasks>   also restart tasks for the connector(s); valid values:
                          all, failed

I was toying with the idea of not requiring an argument to the --tasks flag and defaulting to all in that case, but it seemed a little too complex for users. Still, kcctl restart connector -t file-source does feel noticeably less clunky than kcctl restart connector -t all file-source, so it'd be nice to hear other perspectives on whether that'd be worth supporting.

@C0urante C0urante force-pushed the gh-102 branch 2 times, most recently from 91e2f31 to e2b94d1 Compare July 31, 2023 16:53
Comment on lines +74 to +79
@POST
@Path("/connectors/{name}/restart")
ConnectorStatusInfo restartConnectorAndTasks(
@PathParam("name") String name,
@QueryParam("includeTasks") boolean includeTasks,
@QueryParam("onlyFailed") boolean onlyFailed);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I really wanted to omit the includeTasks parameter here and hardcode it to be true in the REST request, but couldn't find a way to do that. Based on microprofile/microprofile-rest-client#315 it may not even be possible.

@gunnarmorling

Copy link
Copy Markdown
Collaborator

I was toying with the idea of not requiring an argument

Good question. I don't think it's a huge burden for users to specify a value for -t, so I'd go with that. We can still make it optional and add a default later on should we feel that's truly better; while in contrast, we couldn't change it from optional to mandatory, so I'd start with it being a required option, if that makes sense.

@gunnarmorling gunnarmorling merged commit 04bfe0f into kcctl:main Jul 31, 2023
@gunnarmorling

Copy link
Copy Markdown
Collaborator

Merged; thanks, @C0urante!

@C0urante C0urante deleted the gh-102 branch July 31, 2023 18:38
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.

Support new restart semantics in Kafka 3.0

2 participants