Skip to content
This repository has been archived by the owner on Nov 3, 2022. It is now read-only.

Support ExecOptions in dockerman. #243

Merged
merged 10 commits into from
Jul 19, 2019

Conversation

marmistrz
Copy link
Contributor

@marmistrz marmistrz commented Jul 15, 2019

It's easier to add it as an extra field in Command::Exec, but then using session.update(...) from gu-client gets a little more tedious (you need to manually provide the options for each command). An alternative is to add a method to the deployment API, but this will be tricky if we want to run several commands with different working directories (which may actually be the case for gumpi), so I have second thoughts about adding more global state.

@prekucki prekucki self-requested a review July 15, 2019 15:20
@prekucki prekucki self-assigned this Jul 15, 2019
Copy link
Member

@prekucki prekucki left a comment

Choose a reason for hiding this comment

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

camelCase on exec options

#[derive(Clone, Serialize, Deserialize, Hash, Eq, PartialEq, Debug)]
#[serde(rename_all = "camelCase")]
pub enum Command {
Exec {
// return cmd output
executable: String,
args: Vec<String>,
options: ExecOptions,
Copy link
Member

Choose a reason for hiding this comment

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

Change is not backward compatible. it breaks current applications. #[serde(default)] is minimal

@prekucki prekucki merged commit 51b47a6 into golemfactory:release/0.2 Jul 19, 2019
@marmistrz marmistrz deleted the exec_options branch July 19, 2019 15:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants