Also adds custom linter rules to catch use of these functions.
- R1 dagster
Lint Warnings Severity Location Code Message Warning python_modules/dagster/dagster_tests/api_tests/test_launch_scheduled_execution.py:24 W0611 Unused Import
No Test Coverage
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)
can this just be something like time.time(hour=9, minute=30), why does it even care what time it is now
do we need a migration for this? or are we fine with it only kicking in for new users?
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?
dt = pendulum.instance(dt).in_tz("UTC")?
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)
i don't know enough about airflow to understand the implications of this change
pendulum.instance(datetime) instead of datetime.replace?
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
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