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

Allow empty container_map #243

Closed
wants to merge 1 commit into from

Conversation

Georgiy-Tugai
Copy link
Contributor

Fixes dfw exiting immediately if no containers are running.

Fixes dfw exiting immediately if no containers are running.
@pitkley
Copy link
Owner

pitkley commented May 23, 2020

@Georgiy-Tugai thanks for your PR! While the change itself is very simple, I will have to re-familiarize myself with the surrounding logic to see if this makes sense as is.

Right off the bat though I think the change itself makes sense, although I think the return-type of the function should be changed, which would get rid of the .ok_or_else at the call-site.

@Georgiy-Tugai
Copy link
Contributor Author

That's OK! I have almost zero experience with Rust, so I didn't want to mess with the function's return type. 😄

I've tested this on my small Docker deployment and it seems to work fine so far, for what it's worth.

bors bot added a commit that referenced this pull request May 30, 2020
267: Remove special handling for empty container_map r=pitkley a=pitkley

DFW can be used without any containers running, given that it performs
various actions that are not specific to a running container (managing
default policies, handling custom initialization rules).

Thanks to @Georgiy-Tugai for the initial pull-request!

Closes #243.

Co-authored-by: Georgiy Tugai <[email protected]>

Co-authored-by: Pit Kleyersburg <[email protected]>
@bors bors bot closed this in 552e6de May 30, 2020
@bors bors bot closed this in #267 May 30, 2020
@pitkley
Copy link
Owner

pitkley commented May 30, 2020

Hey @Georgiy-Tugai, thanks again for the contribution.

I have taken a look at the change and it definitely made sense! Since I performed some refactorings between when you created the PR and now, I have created a separate PR and included your change in 552e6de (and I took the opportunity to modify the return-type).

This change will be part of v1.2.0. 👍

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.

2 participants