Treat warehouse schedules as in the opscenter-configured timezone. #397
Conversation
1. All warehouse schedule tasks should be marked as running in the configured opscenter timezone. 2. When the task runs, convert the current time and the last task run time to the configured timezone. Then, apply those times against the task schedule and execute the logic per usual. Adds some (contrived) SQL unit tests to validate the logic of this task before we have the admin procedures. Closes sundeck-io#396
| try: | ||
| return timezone(str_tz) | ||
| except UnknownTimeZoneError: | ||
| return timezone("America/Los_Angeles") |
There was a problem hiding this comment.
we can't know the account timezone?
There was a problem hiding this comment.
Ah, here we can probably figured it out from Snowflake. I hadn't thought about checking the snowflake default.
| if (tz is null) then | ||
| tz := 'America/Los_Angeles'; | ||
| end if; | ||
| task_outcome := (select object_insert(:task_outcome, 'opscenter timezone', :tz)); |
There was a problem hiding this comment.
I am not sure why we have two lines here. as in they are basically the same info right?
There was a problem hiding this comment.
Ah, one is recording what the configured timezone for OpsCenter is and the other is recording what the default timezone is for the user's Snowflake account. I thought knowing this might be helpful in debugging issues in the future.
| LANGUAGE JAVASCRIPT | ||
| AS | ||
| $$ | ||
| return Intl.DateTimeFormat().resolvedOptions().timeZone; |
There was a problem hiding this comment.
This returns a timezone identifier like "America/New_York" (or similar).
| -- The task calls this procedure with NULL and lets the procedure figure out the details. | ||
| -- The ability to specify timestamps is only to enable testing. | ||
| if (this_run is NULL) then | ||
| this_run := (select current_timestamp()); | ||
| this_run := (select CONVERT_TIMEZONE(internal.get_current_timezone(), :tz, current_timestamp())); |
There was a problem hiding this comment.
whats the behaviour of this when we pass in an LTZ timezone instead of an NTZ?
There was a problem hiding this comment.
For the 3-arg CONVERT_TIMEZONE function, I don't believe there is any difference if you pass an LTZ or an NTZ (because the timestamp is assumed to be in the timezone specified by the first argument).
I think changing this procedure to accept an NTZ instead of an LTZ is the way we solve the goofiness of the timestamps shifting when given to a procedure. I had been messing around with this over the weekend which works as I expect:
create or replace procedure internal.test_tz(now timestamp_ntz)
returns table(tz_ts TIMESTAMP_NTZ, ts timestamp_ltz, tz_ts2 TIMESTAMP_NTZ, ts2 TIMESTAMP_NTZ)
language sql
as
begin
let res resultset := (
select convert_timezone(internal.get_Current_timezone(), 'America/New_York', current_timestamp()),
current_timestamp(),
convert_timezone(internal.get_current_timezone(), 'America/New_York', :now),
:now);
return table(res);
end;
|
Going to ask for forgiveness and go ahead with the merge without re-approval since I only made a change to the test teardown code. I filed #398 as follow-on. Happy to file others if I've missed a request. |
Adds some (contrived) SQL unit tests to validate the logic of this task before we have the admin procedures.
Closes #396