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

Wait with nothing to wait for waits the whole timeout delay #276

Closed
mgurov opened this issue Aug 29, 2015 · 4 comments
Closed

Wait with nothing to wait for waits the whole timeout delay #276

mgurov opened this issue Aug 29, 2015 · 4 comments

Comments

@mgurov
Copy link
Contributor

mgurov commented Aug 29, 2015

Given wait config with no <log> or <http> entries, the docker:run will report success after waiting for the max allowed time.

Expected: no wait and success (my preference) or configuration error.

Will provide a PR upon request.

@rhuss
Copy link
Collaborator

rhuss commented Aug 30, 2015

You are right, <wait> without any condition should be treated as a configuration error,.

If you only use <time> this is not interpreted as an timeout but as an amount of milliseconds to wait until to continue without error. Only when combined with one of the other conditions it acts as an timeout. This was changed because of https://github.com/rhuss/docker-maven-plugin/issues/173#issuecomment-111870354

@mgurov
Copy link
Contributor Author

mgurov commented Aug 30, 2015

OK, so we do want to wait for the explicitly specified <time> when no condition (log or http) specified.

The presence of the unrelated <shutdown> parameter in the <wait> clause makes strict validation bit clumsy IMHO. Besides we probably don't want to wait on start when only the <shutdown> parameter is present. How about simply skipping the delay after start on no time or conditions specified, as in following table:

<log>/<http>/<url> <time> <effect>
<time> timeout on <log>/<http>/<url> condition
default timeout on <log>/<http>/<url> condition
pause for <time>
don't delay after start

@rhuss
Copy link
Collaborator

rhuss commented Aug 31, 2015

Looks good to me. So, an empty <wait> is essentially a nop, the other behaviour is actually as it is already implemented IIRC.

mgurov added a commit to mgurov/docker-maven-plugin that referenced this issue Sep 6, 2015
…ic8io#276)

+ refactor and test checkers cleanup

Signed-off-by: Mykola Gurov <ngurov@gmail.com>
rhuss pushed a commit that referenced this issue Sep 8, 2015
+ refactor and test checkers cleanup

Signed-off-by: Mykola Gurov <ngurov@gmail.com>
@rhuss
Copy link
Collaborator

rhuss commented Sep 8, 2015

Fixed in branch integration, available in 0.13.4

@rhuss rhuss closed this as completed Sep 8, 2015
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

No branches or pull requests

2 participants