Also adds custom linter rules to catch use of these functions.
Diff Detail
Diff Detail
- Repository
- R1 dagster
- Branch
- utcnow
- Lint
Lint Passed - Unit
No Test Coverage
Event Timeline
Comment Actions
love a good timezone diff. The big thing is that I think we should standardize on pendulum datetimes? (and probably make get_utc_timezone private rather than adding more callsites)
| python_modules/dagster-graphql/dagster_graphql_tests/graphql/setup.py | ||
|---|---|---|
| 864 | can this just be something like time.time(hour=9, minute=30), why does it even care what time it is now | |
| python_modules/dagster/dagster/core/storage/event_log/schema.py | ||
| 12 | do we need a migration for this? or are we fine with it only kicking in for new users? | |
| python_modules/dagster/dagster/core/storage/event_log/sql_event_log.py | ||
| 31 | So this adds get_utc_timezone calls in a bunch of new places - should we settle on using pendulum timezones instead since that is what get_current_datetime_in_utc uses, so that there's a single timezone representation in the codebase? | |
| python_modules/dagster/dagster/utils/__init__.py | ||
| 34 | pendulum.from_timestamp(0)? | |
| 452–453 | dt = pendulum.instance(dt).in_tz("UTC")? | |
| 526 | pendulum.from_timestamp(timestamp)? | |
| python_modules/dagster/dagster/utils/partitions.py | ||
| 55–60 | This is the desired end state and where i'm planning that we get to in 0.10.0 (see https://github.com/dagster-io/dagster/issues/3128 which tracks adding a requirement that you specify a timezone), but changing it right now could break existing users' schedules, if they're assuming that the datetime they specify is in the system timezone I think it's also not true as currently written? on line 112 we still set the timezone to the system timezone if one isn't currently set (via pendulum.now().timezone.name) | |
| python_modules/libraries/dagster-airflow/dagster_airflow/cli.py | ||
| 44 | i don't know enough about airflow to understand the implications of this change | |
| python_modules/libraries/dagster-aws/dagster_aws/cloudwatch/loggers.py | ||
| 125 | pendulum.instance(datetime) instead of datetime.replace? | |
| python_modules/dagster-graphql/dagster_graphql_tests/graphql/setup.py | ||
|---|---|---|
| 864 | i don't have any idea what this test setup is meant to be doing so i don't want to mess with the semantics | |
| python_modules/dagster/dagster/core/storage/event_log/sql_event_log.py | ||
| 31 | i don't understand the logic behind get_utc_timezone at all -- i would define a single constant like dagster.seven.UTC given my druthers | |
| python_modules/dagster/dagster/utils/__init__.py | ||
| 34 | yep | |