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

[CT-2558] [Feature] Make run_started_at an aware Python datetime #7581

Open
3 tasks done
dbeatty10 opened this issue May 9, 2023 · 0 comments
Open
3 tasks done

[CT-2558] [Feature] Make run_started_at an aware Python datetime #7581

dbeatty10 opened this issue May 9, 2023 · 0 comments
Labels
enhancement New feature or request tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality

Comments

@dbeatty10
Copy link
Contributor

Is this your first time submitting a feature request?

  • I have read the expectations for open source contributors
  • I have searched the existing issues, and I could not find an existing issue for this feature
  • I am requesting a straightforward extension of existing dbt functionality, rather than a Big Idea better suited to a discussion

Describe the feature

In the following source code, we can see how run_started_at is a naive(!) Python datetime representing the current time in UTC:

So that the run_started_at can be compared unambiguously with aware timestamps, it should probably be aware as well with one of the two approaches:

  1. dt.now(timezone.utc).isoformat() (offset from UTC)
  2. dt.now().astimezone().isoformat() (offset from system time zone)

There's a good summary here of dbt Python datetimes.

But regardless of deprecation_date for dbt model versions, the fact that it's naive is tech debt that we should pay down, IMO by upgrading run_started_at to an aware timestamp with UTC offset of +00:00.

Describe alternatives you've considered

The most obvious alternative is to leave it as-is (if it's not broke, don't fix it, right?).

I actually don't know if "fixing" this would have any unintended consequences for folks using it like either of these examples:

select
    '{{ run_started_at.strftime("%Y-%m-%d") }}' as date_day
  
from ...
select
    '{{ run_started_at.astimezone(modules.pytz.timezone("America/New_York")) }}' as run_started_est
  
from ...

I would hope that it wouldn't break in either of those cases, but I didn't try them out one way or the other.

Who will this benefit?

One place this could come into play is here:

Are you interested in contributing this feature?

No response

Anything else?

No response

@dbeatty10 dbeatty10 added enhancement New feature or request triage labels May 9, 2023
@github-actions github-actions bot changed the title [Feature] Make run_started_at an aware Python datetime [CT-2558] [Feature] Make run_started_at an aware Python datetime May 9, 2023
@dbeatty10 dbeatty10 added tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality and removed triage labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality
Projects
None yet
Development

No branches or pull requests

1 participant