Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(manager): Make the manager responsible for saving the tasks. #417

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

VoyTechnology
Copy link
Member

This change adds a empty state store which does nothing. It is not yet fully decided what component should be storing the state, so the default is changed for the worker do not save any data.

The role of saving the progress of tasks falls onto manager.

This change adds a empty state store which does nothing. It is not yet
fully decided what component should be storing the state, so the default
is changed for the worker do not save any data.
// fails or do we keep it?
}
}

Ok(())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this and remove the return keywords in the match statement for brevity.

TaskStatus::TASK_PENDING => {
File::create(pending_file_path)
.context(StateErrorKind::PendingTaskWriteFailed)?
.write_all(&task_id.as_bytes())
.context(StateErrorKind::PendingTaskWriteFailed)?;
Ok(())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all of these can be removed if you remove the semicolon on the previous lines

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the following error when I do this:

error[E0308]: match arms have incompatible types
   --> manager/src/state/file.rs:138:17
    |
138 | /                 File::create(pending_file_path)
139 | |                     .context(StateErrorKind::PendingTaskWriteFailed)?
140 | |                     .write_all(&task_id.as_bytes())
141 | |                     .context(StateErrorKind::PendingTaskWriteFailed)?
    | |_____________________________________________________________________^ expected enum `std::result::Result`, found ()
    |
    = note: expected type `std::result::Result<(), state::StateError>`
               found type `()`

Copy link
Member Author

@VoyTechnology VoyTechnology Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would I just wrap the whole statement in Ok(...)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants