Refactored Tink Worker #596
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #596 +/- ##
==========================================
+ Coverage 43.41% 45.66% +2.24%
==========================================
Files 51 52 +1
Lines 3107 3147 +40
==========================================
+ Hits 1349 1437 +88
+ Misses 1673 1624 -49
- Partials 85 86 +1
Continue to review full report at Codecov.
|
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
micahhausler
force-pushed
the
refactor/tink-worker
branch
14 times, most recently
from
6a3a391
to
43c3fed
Compare
micahhausler
commented
Mar 15, 2022
mmlb
reviewed
Mar 16, 2022
micahhausler
force-pushed
the
refactor/tink-worker
branch
from
43c3fed
to
70409cd
Compare
mmlb
suggested changes
Mar 16, 2022
There was a problem hiding this comment.
looks like the only hang up is the logger := log.Test calls should be inside the t.Run to be most effective. 🤔 not even sure what would happen if child test were to fail, pretty sure the logs will still get printed but it'd also include the successful sibling tests too
micahhausler
force-pushed
the
refactor/tink-worker
branch
from
70409cd
to
93110e8
Compare
|
@mmlb Fixed logger initialization |
micahhausler
force-pushed
the
refactor/tink-worker
branch
from
93110e8
to
5d25a11
Compare
micahhausler
force-pushed
the
refactor/tink-worker
branch
3 times, most recently
from
b685e4d
to
2b38d71
Compare
|
@mmlb anything else on this? |
micahhausler
force-pushed
the
refactor/tink-worker
branch
2 times, most recently
from
f5a14df
to
f8617f6
Compare
thebsdbox
previously approved these changes
Mar 30, 2022
3 tasks
mmlb
reviewed
Apr 4, 2022
micahhausler
force-pushed
the
refactor/tink-worker
branch
from
f8617f6
to
2b38d71
Compare
* Create ContainerManager and LogCapturer abstractions * Add tests for new interfaces * Clean up worker method arguments Signed-off-by: Micah Hausler <mhausler@amazon.com>
micahhausler
force-pushed
the
refactor/tink-worker
branch
from
2b38d71
to
5eb8c9b
Compare
mmlb
approved these changes
Apr 4, 2022
Merged
mergify bot
added a commit
that referenced
this pull request
Aug 29, 2022
I'm a maintainer of several other services often related to the Kuberenetes back-end/Kubernetes controllers and I'm taking ownership for a lot of release synchronization making it both appropriate and necessary for me to maintain aspects of the Tink repository. Requirements: - I have reviewed the [community membership guidelines](https://github.com/tinkerbell/proposals/blob/main/proposals/0024/GOVERNANCE.md) - I have [enabled 2FA on my GitHub account](https://github.com/settings/security) - I have subscribed to the [tinkerbell-contributors e-mail list](https://groups.google.com/g/tinkerbell-contributors) - I am actively contributing to 1 or more Tinkerbell subprojects Here is a list of non-trival PRs I have been the primary reviewer on: #596 #628 #614 I have also made a number of code contributions to this repository, here are a few of them: #638 #631 #626 #622 #612 I have also raised various issues and am driving the releasing across Tinkerbell including in this repository. Requesting consideration of expedited responsibilities: yes Sponsors: - @mmlb (Maintainer) - @micahhausler (Maintainer) - @jacobweinstock (Core contributor in other Tinkerbell repositories)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
ContainerManagerandContainerLoggerabstractionsWorkerstruct and initializationWhy is this needed
I was working on #573 and wanted to be able to create a 'virtual worker' with faked-out Docker client calls. This refactor will make it easy immediately test API changes and later refactor the worker logic.
How Has This Been Tested?
How are existing users impacted? What migration steps/scripts do we need?
No impact to existing users
Checklist:
I have: