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

Timer tasks no longer cancelable #322

Closed
m1stermanager opened this issue Sep 16, 2021 · 9 comments
Closed

Timer tasks no longer cancelable #322

m1stermanager opened this issue Sep 16, 2021 · 9 comments
Assignees
Labels
bug Something isn't working hotfix Will provide a hotfix for this issue

Comments

@m1stermanager
Copy link

🐛 Describe the bug
In all durable function documentation referencing timers (including in this repo's code samples) timeout-able orchestrations contain a snippet like:

(from https://github.com/Azure/azure-functions-durable-python/blob/dev/samples/human_interaction/E4_SMSPhoneVerification/__init__.py#L38)

    if not timeout_task.is_completed:
        # All pending timers must be complete or canceled before the function exits.
        timeout_task.cancel()

When using the same technique, there is not a is_completed property, and there is no cancel method. The alternative would appear to be state and change_state respectively.... but I would need some confirmation on this... and some docs would need to be updated (I would be more than willing to open some PRs across the appropriate repos)

Hopefully this is enough detail... I attempted to find some change that removed this method in the history but could not find one, nor a replacement method on the DurableOrchestrationContext but could not.

🤔 Expected behavior
I guess I expected the methods to be present.... if not in the type hints at least on the object, however the code fails as given in examples

Steps to reproduce
when running this locally w/ the following versions according to pip freeze

azure-functions==1.7.2
azure-functions-durable==1.1.1

any attempt to use the timer from a DurableOrchestrationContext.

my code, with extraneous bits removed looks like:

import azure.functions as func
import azure.durable_functions as df

def orchestrator_function(context: df.DurableOrchestrationContext):
    external_task = context.wait_for_external_event("ExternalAction")
    time_limit = context.current_utc_datetime + datetime.timedelta(minutes=5)
    timer_task = context.create_timer(time_limit)

    winner = yield context.task_any([timer_task, external_task])
    if winner == timer_task:
        logger.info('time expired')
    else:
        if timer_task.is_completed:
            timer_task.cancel()

If deployed to Azure
N/A

@davidmrdavid davidmrdavid self-assigned this Sep 20, 2021
@davidmrdavid
Copy link
Collaborator

Hi @m1stermanager,

Thank you so much for reaching out. Indeed, I think you've stumbled upon a recent regression that I accidentally introduced in a recent major refactor of the codebase. We recently changed the way that tasks are represented (we made it more unified) and in that process it seems we dropped these two properties. I'll be adding some validation steps to make sure we don't drop them again. In any case, I can most likely make a patch for this by the end of this week, I'll look to make a release with the patch then! Thanks :)

@davidmrdavid davidmrdavid added bug Something isn't working hotfix Will provide a hotfix for this issue labels Sep 20, 2021
@m1stermanager
Copy link
Author

@davidmrdavid Thanks so much for the response! I'll keep an eye out for that release.

@davidmrdavid
Copy link
Collaborator

Hi @m1stermanager, we just released a new version of this SDK (v1.1.2) which should have patched this issue. Can you please install it and let us know if this issue continues? Thanks!

PyPI link - https://pypi.org/project/azure-functions-durable/1.1.2/
Release notes - https://github.com/Azure/azure-functions-durable-python/releases/tag/v1.1.2

@m1stermanager
Copy link
Author

thanks so much. i'll check this out right now

@m1stermanager
Copy link
Author

looks great to me! thank so much for the quick patch!

@davidmrdavid
Copy link
Collaborator

Great to hear! Please do reach out again if any issues come up, I'll be closing this issue in the meantime :)

@m1stermanager
Copy link
Author

awesome. the only thing i noticed (just now of course), is the type hint on create_timer is still a TaskBase, but that only affects intellisence etc.

Thanks again for the patch!

@davidmrdavid
Copy link
Collaborator

Ah, that's a good catch, thanks! Just so we can track it, could you please file another issue noting this? Thanks again :-)

@m1stermanager
Copy link
Author

oh yeah I can do that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hotfix Will provide a hotfix for this issue
Projects
None yet
Development

No branches or pull requests

2 participants