Skip to content

Fix purge requests without a creation start time - #616

Open
andystaples wants to merge 1 commit into
devfrom
andystaples-fix-purge-default-start-time
Open

Fix purge requests without a creation start time#616
andystaples wants to merge 1 commit into
devfrom
andystaples-fix-purge-default-start-time

Conversation

@andystaples

@andystaples andystaples commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • default purge_instance_history_by to datetime.min when created_time_from is omitted
  • serialize years below 1000 with four digits across platforms
  • add regression coverage for the generated purge URL

The Durable extension intentionally requires createdTimeFrom so a bare collection DELETE cannot become an accidental unfiltered purge. The SDK method explicitly exposes created_time_from as optional, so it now translates that omission into an explicit lower bound instead.

Fixes #560
Fixes #533
Fixes #470

Testing

  • python -m pytest -q tests\models\test_DurableOrchestrationClient.py tests\models\test_RpcManagementOptions.py
  • python -m flake8 azure\durable_functions\models\DurableOrchestrationClient.py azure\durable_functions\models\RpcManagementOptions.py

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ed5b5ae5-2c5e-4422-9786-a23ad1618634
The results of the request to purge history
"""
if created_time_from is None:
created_time_from = datetime.min

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's my understanding that datetime.min results in a date like 0001-01-01T00:00:00.000000Z. One concern I have is whether all the backends are able to correctly interpret such an early date. Some backends, like MSSQL, are sensitive about date ranges and might choke on this. Consider whether something like 2000-01-01T00:00:00.000000Z would be better.

mock_request = MockRequest(
expected_url=f"{RPC_BASE_URL}instances/"
"?createdTimeFrom=0001-01-01T00:00:00.000000Z"
"&runtimeStatus=Running",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: I don't think purging "Running" instances is actually valid, so I suggest that these tests use a valid status filter to avoid confusing readers (or agents) about what a valid use case for purge is.

Comment thread CHANGELOG.md

### Fixed

- `purge_instance_history_by` now supplies the earliest supported creation time when `created_time_from` is omitted, avoiding HTTP 400 responses from the Durable extension.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One risk from this change is that an API call that previously returned a 400 now attempts to delete everything, which can be quite destructive. I'm not as familiar with the original issue, but we should think carefully about whether this risk is worth the benefit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants