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

Feature/followlogsafterstart #1421

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

omyl
Copy link
Contributor

@omyl omyl commented Dec 7, 2020

resolve #1420

(WIP: LogRequestor changes seems to be ugly)

Ondřej Mach added 3 commits December 7, 2020 18:59
@sonarcloud
Copy link

sonarcloud bot commented Dec 7, 2020

@codecov
Copy link

codecov bot commented Dec 7, 2020

Codecov Report

Merging #1421 (7e3f3d0) into master (9f729e2) will decrease coverage by 1.24%.
The diff coverage is 17.77%.

@@             Coverage Diff              @@
##             master    #1421      +/-   ##
============================================
- Coverage     58.79%   57.55%   -1.25%     
+ Complexity     1974     1916      -58     
============================================
  Files           162      162              
  Lines          9014     9040      +26     
  Branches       1362     1358       -4     
============================================
- Hits           5300     5203      -97     
- Misses         3229     3363     +134     
+ Partials        485      474      -11     
Impacted Files Coverage Δ Complexity Δ
...c/main/java/io/fabric8/maven/docker/StartMojo.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...java/io/fabric8/maven/docker/wait/ExitChecker.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...a/io/fabric8/maven/docker/wait/LogWaitChecker.java 83.33% <50.00%> (-10.00%) 6.00 <0.00> (ø)
.../fabric8/maven/docker/access/log/LogRequestor.java 79.54% <60.00%> (+0.53%) 13.00 <2.00> (ø)
...fabric8/maven/docker/AbstractBuildSupportMojo.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-3.00%)
...ven/docker/access/hc/ApacheHttpClientDelegate.java 10.60% <0.00%> (-47.37%) 0.00% <0.00%> (-17.00%)
...c/main/java/io/fabric8/maven/docker/BuildMojo.java 0.00% <0.00%> (-32.10%) 0.00% <0.00%> (-12.00%)
...va/io/fabric8/maven/docker/AbstractDockerMojo.java 10.61% <0.00%> (-6.20%) 8.00% <0.00%> (-3.00%)
...ic8/maven/docker/config/AssemblyConfiguration.java 75.38% <0.00%> (-4.62%) 14.00% <0.00%> (-5.00%)
...config/handler/property/PropertyConfigHandler.java 76.56% <0.00%> (-4.27%) 124.00% <0.00%> (-11.00%)
... and 6 more

@rohanKanojia rohanKanojia self-requested a review March 7, 2021 04:59
@rohanKanojia
Copy link
Member

@omyl : Hi, Is this PR still a Work in Progress?

@omyl
Copy link
Contributor Author

omyl commented Mar 7, 2021

@omyl : Hi, Is this PR still a Work in Progress?

Hi, last changes in last commit (most imporatnt) are not covered by tests - and currently I'm out-of-time to do that.
Functionality changes are fully finished.

@rhuss
Copy link
Collaborator

rhuss commented Mar 7, 2021

@omyl thanks for the PR ! So, if I understood your last comment correctly, you don't plan to continue to work on the PR but would be happy if we would pick up the fix ? (e.g. some cleanup would be required, too, to remove som debug log info).

Could you please briefly explain what the PR is actually doing and what it is fixing ? So a 2-3 sentence summary of the change would be awesome.

@omyl
Copy link
Contributor Author

omyl commented Mar 7, 2021

@rhuss - I have updated description of related issue too

  1. to allow complex handling of started container in "running phase" (i.e. wait for all containers terminates) collect full container object (instead of id only)
  2. to allow plugin process stop in "followlogs" case (relevant when all containers terminates) - implement "all exit code" checker (and use implemented WaitUntil feature)
  3. to minimalize "freeze" of plugin process move posiible problematic invocations into separate threads and handle exception for impossible situation

@rhuss
Copy link
Collaborator

rhuss commented Mar 7, 2021

thanks ! I'm not sure whether I can dive into this PR soon as it bit involved (and my time budget for maintaining this plugin is super minimal), but @rohanKanojia if you want to pick this up fee free ;-)

otherwise we would need to park the PR until the next round.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stop processing when followlogs-containers terminats
3 participants