Skip to content

Conversation

@hectorcast-db
Copy link
Contributor

@hectorcast-db hectorcast-db commented May 21, 2025

What changes are proposed in this pull request?

Add custom Duration and Timestamp classes with nanosecond support and helper methods

How is this tested?

Added tests

NO_CHANGELOG=true

Comment on lines 28 to 42
def __init__(self, seconds: int = 0, nanoseconds: int = 0) -> None:
"""Initialize a Duration with seconds and nanoseconds.

Args:
seconds: Number of seconds
nanoseconds: Number of nanoseconds (0-999999999)

Raises:
TypeError: If seconds or nanoseconds are not integers
ValueError: If nanoseconds is not between 0 and 999999999
"""
if not isinstance(seconds, int):
raise TypeError("seconds must be an integer")
if not isinstance(nanoseconds, int):
raise TypeError("nanoseconds must be an integer")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this type check when we explicitly stated that in the function arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The type in the arguments are only "hints" for the linter, but python does not enforce them.

self.nanoseconds = nanoseconds

@classmethod
def from_timedelta(cls, td: timedelta) -> "Duration":
Copy link
Contributor

Choose a reason for hiding this comment

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

This is returning "Duration" with quotes. Is this expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. In Python you cannot use a type before introduced. This is a workaround introduced at some point:

https://peps.python.org/pep-0484/#forward-references

from datetime import datetime, timedelta, timezone
from decimal import Decimal

_LOG = logging.getLogger("databricks.sdk")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

Comment on lines 1 to 4
"""Common types for the Databricks SDK.

This module provides common types used by different APIs.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we actually move this in databricks/sdk/core/common? I'd like to reserve databricks/sdk/common for SDK-Mod.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to databricks/sdk/common_types/common.py since moving it to core creates circular dependencies witth the core.py file.

@hectorcast-db hectorcast-db force-pushed the hectorcast-db/duration-timestamp branch from e9c4694 to 389e0c6 Compare June 16, 2025 10:00
@github-actions
Copy link

github-actions bot commented Jul 4, 2025

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-py

Inputs:

  • PR number: 975
  • Commit SHA: 698ba478434ca95d8b300a9a03230cc8fdfbbce1

Checks will be approved automatically on success.

Copy link
Contributor

@renaudhartert-db renaudhartert-db left a comment

Choose a reason for hiding this comment

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

This is not relevant anymore right?

@hectorcast-db
Copy link
Contributor Author

Not relevant.

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.

4 participants