workers-rs icon indicating copy to clipboard operation
workers-rs copied to clipboard

Add DO Alarm API

Open fisherdarling opened this issue 3 years ago • 2 comments

This PR implements the DO Alarm API based off of the signatures found in the storage docs.

I think there's a bug in the way I've implemented the binding for getAlarm. When an alarm does not exist, I expect getAlarm to return null and in rust Ok(None), however I seem to be getting an error instead. When the alarm does exist (set_alarm immediately followed by a get_alarm) I get the expected output.

I haven't been able to test that the alarm does trigger, however everything compiles and it looks like the generated shim has the required methods. I'll be testing later tonight.

Also, I have set_alarm take in a DateTime<Utc> whereas get_alarm returns an i64.

  • Thoughts on having both a set_alarm and set_alarm_datetime?
  • Should there also be a get_alarm_datetime for convenience?
  • Should we have inputs and outputs just be DateTime<Utc>s?

fisherdarling avatar May 18 '22 22:05 fisherdarling

Some changes needed, but overall looks good. The new methods need to have option arguments and we need to add testing for this in worker-sandbox. Miniflare support for DO alarms seems a bit rough right now, but should be good enough for us to get basic testing.

Sorry for the delay on the review.

@zebp Sounds good! I like the idea of using a trait for the argument. No worries on the delayed review.

I have some family visiting me right now so I'm aiming to get the additional work done sometime next Monday or Tuesday.

fisherdarling avatar Jun 06 '22 14:06 fisherdarling

@zebp Currently stuck on testing. A few days ago I tried to add some and I don't think I was able to get it to work locally. Are my testing changes in the right direction?

fisherdarling avatar Jun 12 '22 11:06 fisherdarling

Finished up and rebased this PR, thanks for the contribution!

zebp avatar Aug 18 '22 18:08 zebp