You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by "pbacsko (via GitHub)" <gi...@apache.org> on 2023/05/31 14:26:40 UTC
[GitHub] [yunikorn-core] pbacsko opened a new pull request, #559: [YUNIKORN-1774] Event cache: misc cleanup
pbacsko opened a new pull request, #559:
URL: https://github.com/apache/yunikorn-core/pull/559
### What is this PR for?
Misc cleanup in the current event implementation:
- "event system" instead of "event cache"
- remove interfaces
- add extra event sending layer in `Application`
### What type of PR is it?
* [ ] - Bug Fix
* [x] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-1774
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #559: [YUNIKORN-1774] Event cache: misc cleanup
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #559:
URL: https://github.com/apache/yunikorn-core/pull/559#discussion_r1217714281
##########
pkg/events/event_system.go:
##########
@@ -29,39 +29,45 @@ import (
// need to change for testing
var defaultEventChannelSize = 100000
-var cache *EventCache
+var ev EventSystem
-type EventCache struct {
- Store EventStore // storing eventChannel
+type EventSystem interface {
Review Comment:
Changed this to an interface so that we can mock easier when needed
##########
pkg/events/event_system.go:
##########
@@ -29,39 +29,45 @@ import (
// need to change for testing
var defaultEventChannelSize = 100000
-var cache *EventCache
+var ev EventSystem
-type EventCache struct {
- Store EventStore // storing eventChannel
+type EventSystem interface {
Review Comment:
Changed this to an interface so that we can mock it easier when needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko commented on pull request #559: [YUNIKORN-1774] Event cache: misc cleanup
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on PR #559:
URL: https://github.com/apache/yunikorn-core/pull/559#issuecomment-1573347561
> I think we should add as many tests as possible to `events.go` which will be helpful when new code gets introduced as part of YUNIKORN-1628.
I'll add some unit tests to increase coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] codecov[bot] commented on pull request #559: [YUNIKORN-1774] Event cache: misc cleanup
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #559:
URL: https://github.com/apache/yunikorn-core/pull/559#issuecomment-1570356632
## [Codecov](https://app.codecov.io/gh/apache/yunikorn-core/pull/559?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#559](https://app.codecov.io/gh/apache/yunikorn-core/pull/559?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (3279855) into [master](https://app.codecov.io/gh/apache/yunikorn-core/commit/c5d26a841d1f9e240620ffa6d8b805b163512bbc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (c5d26a8) will **decrease** coverage by `0.02%`.
> The diff coverage is `61.81%`.
```diff
@@ Coverage Diff @@
## master #559 +/- ##
==========================================
- Coverage 74.84% 74.82% -0.02%
==========================================
Files 70 71 +1
Lines 11523 11534 +11
==========================================
+ Hits 8624 8630 +6
- Misses 2595 2600 +5
Partials 304 304
```
| [Impacted Files](https://app.codecov.io/gh/apache/yunikorn-core/pull/559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [pkg/scheduler/objects/events.go](https://app.codecov.io/gh/apache/yunikorn-core/pull/559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2V2ZW50cy5nbw==) | `35.48% <35.48%> (ø)` | |
| [pkg/scheduler/objects/application.go](https://app.codecov.io/gh/apache/yunikorn-core/pull/559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `65.91% <66.66%> (+0.87%)` | :arrow_up: |
| [pkg/events/event\_publisher.go](https://app.codecov.io/gh/apache/yunikorn-core/pull/559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2V2ZW50cy9ldmVudF9wdWJsaXNoZXIuZ28=) | `100.00% <100.00%> (ø)` | |
| [pkg/events/event\_store.go](https://app.codecov.io/gh/apache/yunikorn-core/pull/559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2V2ZW50cy9ldmVudF9zdG9yZS5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/events/event\_system.go](https://app.codecov.io/gh/apache/yunikorn-core/pull/559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2V2ZW50cy9ldmVudF9zeXN0ZW0uZ28=) | `92.50% <100.00%> (ø)` | |
... and [1 file with indirect coverage changes](https://app.codecov.io/gh/apache/yunikorn-core/pull/559/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] pbacsko closed pull request #559: [YUNIKORN-1774] Event cache: misc cleanup
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko closed pull request #559: [YUNIKORN-1774] Event cache: misc cleanup
URL: https://github.com/apache/yunikorn-core/pull/559
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org