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

Implementation of UpdateJobTimeoutCommand #682

Conversation

LennartKleymann
Copy link
Contributor

Added a command for updating the timeout of a job

closes #681

@CLAassistant
Copy link

CLAassistant commented May 22, 2024

CLA assistant check
All committers have signed the CLA.

@LennartKleymann LennartKleymann force-pushed the 681-add-update-job-timeout-command branch from feb37b4 to f59c5e0 Compare May 22, 2024 22:25
@LennartKleymann LennartKleymann force-pushed the 681-add-update-job-timeout-command branch from f59c5e0 to 7b69dc7 Compare May 22, 2024 22:41
Copy link
Collaborator

@ChrisKujawa ChrisKujawa left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for your great contribution @LennartKleymann 🚀

There are only smaller things we have to add/do and then we can merge it. Actually, it is just to add a separate test class for the command, and clarify the timeout conversion. :)

Thanks for your time and effort!

Client/IZeebeClient.cs Outdated Show resolved Hide resolved
Client/Api/Commands/IUpdateJobTimeoutCommandSteps.cs Outdated Show resolved Hide resolved
Client/Impl/Commands/UpdateJobTimeoutCommand.cs Outdated Show resolved Hide resolved
Comment on lines +146 to +147
yield return new TestCaseData(
new UpdateJobTimeoutRequest()
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍🏼 Great that you added here an test case as well.

As this is only for the SendWithRetry test right now we also need some more tests for the happy path to check the expected requests, and responses. Like we do with the other commands. Take as example this https://github.com/camunda-community-hub/zeebe-client-csharp/blob/main/Client.UnitTests/UpdateRetriesTest.cs
What we also need is to add

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added tests to check expected requests

Copy link
Collaborator

@ChrisKujawa ChrisKujawa left a comment

Choose a reason for hiding this comment

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

Great! Thank you for your efforts 👍

@ChrisKujawa ChrisKujawa merged commit 6c7940c into camunda-community-hub:main May 27, 2024
5 checks passed
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.

I can use the client to update a timeout of a job
3 participants