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

Don't fail build if volume-clean fails. #788

Merged
merged 4 commits into from
Nov 4, 2017
Merged

Conversation

KedneckInc
Copy link

This fix is admittedly simplistic. It does not take into account any of the other possible causes for an exception. However, if you are using volume-clean, chances are you're using volume-create, or many of the other goals provided by DMP. In that case, you will almost certainly receive errors pointing you at the
real problem.

@codecov
Copy link

codecov bot commented May 25, 2017

Codecov Report

Merging #788 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master     #788      +/-   ##
============================================
- Coverage     50.98%   50.97%   -0.02%     
+ Complexity     1213     1212       -1     
============================================
  Files           140      140              
  Lines          7129     7129              
  Branches        956      956              
============================================
- Hits           3635     3634       -1     
  Misses         3178     3178              
- Partials        316      317       +1
Impacted Files Coverage Δ Complexity Δ
...ven/docker/access/hc/DockerAccessWithHcClient.java 14.04% <0%> (ø) 12 <0> (ø) ⬇️
...ain/java/io/fabric8/maven/docker/util/EnvUtil.java 66.21% <0%> (-0.68%) 42% <0%> (-1%)

@rhuss
Copy link
Collaborator

rhuss commented Nov 4, 2017

Sorry, for coming back to this PR that late. However, it seems to be broken to me when you look into the list of changes and touched files. The conflicts are because we moved on, but the original changeset is still too huge to even review it.

I'm going to close this PR therefore, it would be awesome if you create a new PR. Alternatively, if you describe the problem in a GitHub issue we could try to find a solution together.

Again sorry for the very late response.

@rhuss rhuss closed this Nov 4, 2017
@rhuss rhuss reopened this Nov 4, 2017
This fix is admittedly simplistic.  It does not take into account any of the
other possible causes for an exception.  However, if you are using volume-clean,
chances are you're using volume-create, or many of the other goals provided by
DMP.  In that case, you will almost certainly receive errors pointing you at the
real problem.
@rhuss
Copy link
Collaborator

rhuss commented Nov 4, 2017

I cherry picked your change now, should be clean again.

@rhuss
Copy link
Collaborator

rhuss commented Nov 4, 2017

I just allowed 404 as a valid response code for a volume delete operation. This is returned when the volume does not exists. I think this is the better solution.

@rhuss rhuss merged commit 170d7d5 into fabric8io:master Nov 4, 2017
@KedneckInc
Copy link
Author

Wunderbar! I'll give it a look because I finally got back to that part of the project!

@KedneckInc
Copy link
Author

Sorry I took so long to get back to this. Yes, this definitely fixed the problem. Many Thanks! Now our developers don't have to manually clean up the old volumes!

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