azriel91 / peace

Zero Stress Automation
https://peace.mk
Apache License 2.0
110 stars 1 forks source link

`Item::apply_dry` is not designed to work with non-existent state #196

Open azriel91 opened 1 month ago

azriel91 commented 1 month ago

Item::apply_dry will not work for cases like the following:

  1. FileDownloadItem downloads a tar file.
  2. TarXItem extracts the file.

Currently TarXApplyFns::apply_dry just returns state_goal.clone() as the "successful state", rather than attempting to read the file, and returning whether the dry run fails or signal that the returned state is inaccurate because the file to extract doesn't exist yet.

We need to decide to settle on one of the following:

  1. Keep the API as is -- item implementors cannot assume params / data are usable. Errors returned from each Item count as an execution failure.
  2. Change the signature to know which values are RealOrExample::Real(T) vs RealOrExample::Example(T), and item implementors choose to read external resources for Real<T> variants.
  3. Change the implementation of ItemWrapper::ensure_prepare to call state_goal_try_exec when it is for a dry run, and only call apply_dry if state_goal managed to be fetched.

Tasks

  1. [ ] Determine the contract, update Item::apply_dry.
  2. [ ] Fix ValueResolutionMode::ApplyDry and peace_data::marker::apply_dry::ApplyDry docs.
  3. [ ] Update implementations for all *Items.