deprecate schedule_notify_task #373
Conversation
pallets/automation-time/src/lib.rs
Outdated
| // the extrinsic is disabled so no one can schedule that kind of tasks, just old tasks | ||
| // TODO: Find out turing staging number and initalize accordingly | ||
| // This is the last block we have this event | ||
| let target_block: u32 = 1244899; |
There was a problem hiding this comment.
@chrisli30 @imstar15 this number is for staging and maybe in-correct.
But just want to get feedback on the direction Im doing is right with this? What is our strategy so far to handle removing functionality and backward compatible when re-syncing and execute code in old blocks?
There was a problem hiding this comment.
Good question. The blockchain itself will run data migration right before new runtime code during an upgrade, so the data needs to be correctly transformed. In order to achieve that, as long as there’s data change in storage, migration could needs to follow.
We write the migration code and use try-runtime script to verify the correctness.
After the release of the code, those one-time data migration needs to be removed so they don’t run in the next release. Here’s the most recent example, #369
v9n
force-pushed
the
remove-schedule_notify
branch
from
25826ed
to
d6bd7dd
Compare
v9n
changed the title
pallets/automation-time/src/tests.rs
Outdated
| @@ -38,12 +38,16 @@ const LAST_BLOCK_TIME: u64 = START_BLOCK_TIME / 1_000; | |||
| #[test] | |||
| fn schedule_invalid_time() { | |||
There was a problem hiding this comment.
Since all timeAutomation call has two kinds of scheduling, Fixed and Recurring, we should duplicate time related tests and test both types.
For Fixed, multiple timestamps need always to be tested since the input is an array, Vec
For Recurring, only one timestamp needs to be tested, but there’s a second parameter frequency to test as well, { next_execution_time: UnixTime, frequency: Seconds}.
|
@imstar15 could you review this PR as well? |
|
@chrisli30 @imstar15 I had addressed all the comments. I tried to add comments per test to a hard to understand un-clear test case. A few tests are duplicated to check recurring schedule logic as well. ready to review agian. |
This PR remove
schedule_notify_taskextrinsic. This extrinsic is also used to setup and run test for many scenario so i migrated them to dynamic dispatch call and assert their behaviour.