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

Requested Changes for current release #78

Merged
merged 4 commits into from
Nov 15, 2021
Merged

Conversation

cdr-chakotay
Copy link
Collaborator

Made requested changes on the current release:

  • simplified retry mechanism, therefore:
    -- counting logic was placed in qualifiedForRetry()
    -- a new method timeoutBeforeRetry(), which performs the wait before the retry, was implemented

Requested additional changes are:

  • custom timeout times, handled by the Transloadit.class
  • renamed the getAssemblyID method.

Additional changes:
jUnit Tests for all new methods

- simplified retry mechanism, therefore:
-- counting logic was placed in qualifiedForRetry()
-- a new method timeoutBeforeRetry(), which performs the wait before the retry, was implemented

Requested additional changes are:
- custom timout times,handled by the Transloadit.class
- renamed the getAssemblyID method.

Additional changes:
jUnit Tests for all new methods
@cdr-chakotay cdr-chakotay added enhancement sdks Integrations for Transloadit's API labels Oct 21, 2021
@cdr-chakotay cdr-chakotay self-assigned this Oct 21, 2021
* @param timeout in ms
* @throws LocalOperationException if provided timeout is smaller than 0
*/
public void setTimeoutRetry(int timeout) throws LocalOperationException {
Copy link
Member

Choose a reason for hiding this comment

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

I think the terminology is a bit off here. A timeout is a time period after with an operation is cancelled if it hasn't finished before the period's end. The term retry delay is probably better suited since it is the period we are waiting between retry attempts. Does that make sense? Apologies if this causes you to rename all the methods again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll go over that right now

Copy link
Member

@Acconut Acconut left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the latest updates!

@Acconut Acconut merged commit 67bdfcd into master Nov 15, 2021
@Acconut Acconut deleted the Fixes_for_current_release branch November 15, 2021 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement sdks Integrations for Transloadit's API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants