Rework timestamp utilization for timezone consistency #3314

stefannica · 2025-01-23T19:11:42Z

Describe changes

Following the previous PR (#3265) that replaced deprecated datetime.utcnow() with datetime.now(timezone.utc), some follow-up is required to ensure we use timezone-aware timestamps appropriately:

no timezone aware timestamps must be stored in the database. The convention is that the database stores only timezone-naive UTC timestamps
comparisons and subtraction operations between timezone-aware and timezone-naive operands result in errors, so they must be avoided

This PR introduces a few time utils to make it easier to deal with timezones. It also goes through all occurrences of datetime.now(timezone.utc) and re-evaluates whether a timezone-aware timestamp is indeed needed and reverts some of the decisions in the previous PR if that's not the case.

Pre-requisites

Please ensure you have done the following:

I have read the CONTRIBUTING.md document.
I have added tests to cover my changes.
I have based my new branch on develop and the open PR is targeting develop. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.
IMPORTANT: I made sure that my changes are reflected properly in the following resources:
- ZenML Docs
- Dashboard: Needs to be communicated to the frontend team.
- Templates: Might need adjustments (that are not reflected in the template tests) in case of non-breaking changes and deprecations.
- Projects: Depending on the version dependencies, different projects might get affected.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Other (add details above)

coderabbitai · 2025-01-23T19:11:51Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

stefannica · 2025-01-23T19:18:12Z

src/zenml/cli/stack.py

@@ -1575,7 +1575,7 @@ def deploy(
        ):
            raise click.Abort()

-        date_start = datetime.now(timezone.utc)
+        date_start = utc_now_tz_aware()


Verified that this should be TZ aware.

src/zenml/integrations/aws/orchestrators/sagemaker_orchestrator.py

stefannica · 2025-01-23T19:23:28Z

src/zenml/integrations/aws/service_connectors/aws_service_connector.py

@@ -66,6 +66,7 @@
 )
 from zenml.utils.enum_utils import StrEnum
 from zenml.utils.secret_utils import PlainSerializedSecretStr
+from zenml.utils.time_utils import utc_now_tz_aware


All timestamps in this module were TZ aware and must be TZ aware

stefannica · 2025-01-23T19:24:40Z

src/zenml/integrations/azure/service_connectors/azure_service_connector.py

-        )
-
-        self.expires_on = int(expires_at.timestamp())
+        self.expires_on = int(to_local_tz(expires_at).timestamp())


This was not changed in the previous PR. It was in fact incorrect and fixed now to convert the time properly.

stefannica · 2025-01-23T19:26:15Z

src/zenml/integrations/azure/service_connectors/azure_service_connector.py

-            now = datetime.datetime.now(datetime.timezone.utc)
-            expires_at = expires_at.replace(tzinfo=datetime.timezone.utc)

            # check if the token expires in the near future
-            if expires_at > now + datetime.timedelta(


This was not changed in the previous PR, it's just a rework for cleaner code.

stefannica · 2025-01-23T19:27:03Z

src/zenml/integrations/gcp/service_connectors/gcp_service_connector.py

-            expires_at = expires_at.replace(tzinfo=datetime.timezone.utc)
+
            # check if the token expires in the near future
-            if expires_at > now + datetime.timedelta(
+            if expires_at > utc_now(tz_aware=expires_at) + datetime.timedelta(


This was not changed in the previous PR, it's just a rework for cleaner code.

src/zenml/utils/time_utils.py

src/zenml/zen_stores/schemas/base_schemas.py

src/zenml/zen_server/zen_server_api.py

schustmi · 2025-01-24T14:49:36Z

src/zenml/zen_stores/sql_zen_store.py

@@ -1743,7 +1744,7 @@ def _update_last_user_activity_timestamp(
            settings = self._get_server_settings(session=session)

            if last_user_activity < settings.last_user_activity.replace(
-                tzinfo=timezone.utc
+                tzinfo=None


This is coming from the DB and should already be a naive datetime, correct? (I guess nothing wrong with still replacing it, just for my understanding)

Yes, correct. I can remove this replace call altogether now.

schustmi · 2025-01-24T14:50:57Z

tests/integration/examples/great_expectations/steps/importer.py

-    df["target"] = plates.target
-    plates_columns = dict(zip(plates.feature_names, FULL_FEATURE_NAMES))
-    df.rename(columns=plates_columns, inplace=True)
+    url = "https://archive.ics.uci.edu/ml/machine-learning-databases/00198/Faults.NNA"


Do we need this change or can we just wait for openml to be up again?
See openml/openml.org#343

Let's just fix this, and we can revert the change later if needed. I'm sick and tired of looking at CI errors.

stefannica requested review from schustmi and bcdurak January 23, 2025 19:11

github-actions bot added the internal To filter out internal PRs and issues label Jan 23, 2025

stefannica added run-slow-ci and removed internal To filter out internal PRs and issues labels Jan 23, 2025

github-actions bot added the bug Something isn't working label Jan 23, 2025

stefannica added 2 commits January 23, 2025 20:12

Create time utils module

78ced86

Rework use of timezone naive and aware operations to be consistent

c41c43e

stefannica force-pushed the bug/fix-utc-timestamps branch from 8dcff29 to c41c43e Compare January 23, 2025 19:12

stefannica commented Jan 23, 2025

View reviewed changes

src/zenml/integrations/aws/orchestrators/sagemaker_orchestrator.py Outdated Show resolved Hide resolved

stefannica commented Jan 23, 2025

View reviewed changes

stefannica added 2 commits January 24, 2025 09:26

Fix sagemaker

b8b5341

Remove omitted timestamp conversion

118bee1

schustmi requested changes Jan 24, 2025

View reviewed changes

src/zenml/utils/time_utils.py Outdated Show resolved Hide resolved

src/zenml/zen_stores/schemas/base_schemas.py Outdated Show resolved Hide resolved

src/zenml/zen_server/zen_server_api.py Show resolved Hide resolved

Apply code-review suggestions

5aff0e5

stefannica requested a review from schustmi January 24, 2025 09:37

schustmi approved these changes Jan 24, 2025

View reviewed changes

stefannica added 4 commits January 24, 2025 11:24

Fix sagemaker once more

11fb483

Fix incorrect astimezone use

f204b7c

Fix activity timestamp updates

26395a4

Fix the great expectations integration test

b0a2904

schustmi requested changes Jan 24, 2025

View reviewed changes

stefannica requested a review from schustmi January 24, 2025 16:46

schustmi approved these changes Jan 24, 2025

View reviewed changes

stefannica merged commit ed1a2b6 into develop Jan 24, 2025
69 checks passed

stefannica deleted the bug/fix-utc-timestamps branch January 24, 2025 16:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework timestamp utilization for timezone consistency #3314

Rework timestamp utilization for timezone consistency #3314

stefannica commented Jan 23, 2025 •

edited

Loading

coderabbitai bot commented Jan 23, 2025

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

stefannica Jan 23, 2025

stefannica Jan 23, 2025 •

edited

Loading

stefannica Jan 23, 2025 •

edited

Loading

stefannica Jan 23, 2025

stefannica Jan 23, 2025

schustmi Jan 24, 2025

stefannica Jan 24, 2025

schustmi Jan 24, 2025

stefannica Jan 24, 2025

Rework timestamp utilization for timezone consistency #3314

Rework timestamp utilization for timezone consistency #3314

Conversation

stefannica commented Jan 23, 2025 • edited Loading

Describe changes

Pre-requisites

Types of changes

coderabbitai bot commented Jan 23, 2025

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

stefannica Jan 23, 2025

Choose a reason for hiding this comment

stefannica Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

stefannica Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

stefannica Jan 23, 2025

Choose a reason for hiding this comment

stefannica Jan 23, 2025

Choose a reason for hiding this comment

schustmi Jan 24, 2025

Choose a reason for hiding this comment

stefannica Jan 24, 2025

Choose a reason for hiding this comment

schustmi Jan 24, 2025

Choose a reason for hiding this comment

stefannica Jan 24, 2025

Choose a reason for hiding this comment

stefannica commented Jan 23, 2025 •

edited

Loading

stefannica Jan 23, 2025 •

edited

Loading

stefannica Jan 23, 2025 •

edited

Loading