Skip to content

Fix vacuum_lakehouse_tables retention period formatting for single-digit hours - #1309

Merged
Michael Kovalsky (m-kovalsky) merged 2 commits into
mainfrom
copilot/fix-retention-period-format
Aug 4, 2026
Merged

Fix vacuum_lakehouse_tables retention period formatting for single-digit hours#1309
Michael Kovalsky (m-kovalsky) merged 2 commits into
mainfrom
copilot/fix-retention-period-format

Conversation

Copilot AI commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

vacuum_lakehouse_tables() could generate an invalid retention_period when retain_n_hours % 24 < 10 (for example 2:1:00:00), which fails downstream validation requiring d:hh:mm:ss. This change ensures the hours segment is always zero-padded to two digits.

  • Root cause and behavior

    • retention_period was built with an unpadded hour component in the vacuum helper path.
    • Values with single-digit hours produced invalid format and raised a ValueError.
  • Code change

    • Updated retention period string construction to enforce two-digit hours:
      # before
      retention_period = f"{retain_n_hours // 24}:{retain_n_hours % 24}:00:00"
      
      # after
      retention_period = f"{retain_n_hours // 24}:{retain_n_hours % 24:02d}:00:00"
  • Regression coverage

    • Added a focused lakehouse unit test that verifies retain_n_hours=49 is translated to 2:01:00:00 before calling run_table_maintenance.

Copilot AI changed the title [WIP] Fix retention_period format for hours less than 10 Fix vacuum_lakehouse_tables retention period formatting for single-digit hours Aug 3, 2026
@m-kovalsky
Michael Kovalsky (m-kovalsky) marked this pull request as ready for review August 4, 2026 05:48
Copilot AI lite review requested due to automatic review settings August 4, 2026 05:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes vacuum_lakehouse_tables() so that the generated retention_period string always conforms to the required d:hh:mm:ss format by zero-padding the hours component when retain_n_hours % 24 is a single digit.

Changes:

  • Zero-pad the hours segment in the retention_period string passed to run_table_maintenance.
  • Add a regression unit test asserting retain_n_hours=49 produces 2:01:00:00.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/test_lakehouse.py Adds a regression test to ensure single-digit hour values are zero-padded in retention_period.
src/sempy_labs/lakehouse/_lakehouse.py Updates vacuum_lakehouse_tables() to format retention_period hours with :02d.
Suppressed comments (1)

src/sempy_labs/lakehouse/_lakehouse.py:244

  • vacuum_lakehouse_tables() unconditionally computes and passes retention_period using retain_n_hours, but retain_n_hours is optional (default None) and the docstring says that when it’s not specified the table’s default retention should be used. As written, calling vacuum_lakehouse_tables() with the default retain_n_hours=None will raise a TypeError (NoneType in ///%) and will never allow the “use default retention” behavior.
        run_table_maintenance(
            table_name=table_name,
            vacuum=True,
            retention_period=f"{retain_n_hours // 24}:{retain_n_hours % 24:02d}:00:00",
            schema=schema_name,
            lakehouse=lakehouse,
            workspace=workspace,
        )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

Wrong format for retention_period if retain_n_hours%24 < 10

4 participants