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

Added env's to be able to configure timouts #165

Conversation

Zyqsempai
Copy link
Contributor

Signed-off-by: Boris Popovschi [email protected]

Issue #164

Description of changes:
Added FIRECRACKER_INIT_TIMEOUT_SECONDS env to configure init timeouts and FIRECRACKER_REQUEST_TIMEOUT_MILLISECONDS for request timeouts.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Zyqsempai
Copy link
Contributor Author

@nmeyerhans @xibz @sipsma PTAL

Copy link
Contributor

@samuelkarp samuelkarp left a comment

Choose a reason for hiding this comment

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

A few really minor comments, but the code otherwise looks pretty good.

firecracker.go Outdated Show resolved Hide resolved
machine.go Outdated Show resolved Hide resolved
utils.go Outdated Show resolved Hide resolved
utils.go Outdated Show resolved Hide resolved
@Zyqsempai Zyqsempai force-pushed the 164-added-configurable-timeouts branch from 06db74e to f04d482 Compare December 19, 2019 10:15
@Zyqsempai
Copy link
Contributor Author

@samuelkarp Done.

Makefile Outdated Show resolved Hide resolved
utils.go Show resolved Hide resolved
utils.go Show resolved Hide resolved
utils.go Show resolved Hide resolved
.buildkite/pipeline.yml Outdated Show resolved Hide resolved
@Zyqsempai Zyqsempai force-pushed the 164-added-configurable-timeouts branch 3 times, most recently from 2db1d36 to 88fc389 Compare December 20, 2019 15:10
@Zyqsempai
Copy link
Contributor Author

Guys, PTAL

Copy link
Contributor

@nmeyerhans nmeyerhans left a comment

Choose a reason for hiding this comment

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

Minor changes requested, otherwise LGTM.

.buildkite/pipeline.yml Outdated Show resolved Hide resolved
.buildkite/pipeline.yml Outdated Show resolved Hide resolved
firecracker.go Outdated Show resolved Hide resolved
@Zyqsempai Zyqsempai force-pushed the 164-added-configurable-timeouts branch from 88fc389 to 5166432 Compare December 23, 2019 13:13
@Zyqsempai
Copy link
Contributor Author

Take a final look guys;)

Copy link
Contributor

@nmeyerhans nmeyerhans left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@nmeyerhans nmeyerhans merged commit 5166432 into firecracker-microvm:master Dec 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants