You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "suddendust (via GitHub)" <gi...@apache.org> on 2023/12/04 10:18:51 UTC
[PR] Retrofit BaseClusterIntegrationTest to Support Multiple Pinot Clusters [pinot]
suddendust opened a new pull request, #12090:
URL: https://github.com/apache/pinot/pull/12090
Old methods have been preserved for backward compatibility.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Retrofit BaseClusterIntegrationTest to Support Multiple Pinot Clusters [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12090:
URL: https://github.com/apache/pinot/pull/12090#issuecomment-1838422470
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12090?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`b9ed378`)](https://app.codecov.io/gh/apache/pinot/commit/b9ed378355acc5313ebc1031510671f7045f29a9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.65% compared to head [(`73df9db`)](https://app.codecov.io/gh/apache/pinot/pull/12090?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 46.85%.
> Report is 4 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12090 +/- ##
=============================================
- Coverage 61.65% 46.85% -14.80%
+ Complexity 1152 944 -208
=============================================
Files 2389 1791 -598
Lines 129819 94130 -35689
Branches 20082 15211 -4871
=============================================
- Hits 80036 44107 -35929
- Misses 43958 46890 +2932
+ Partials 5825 3133 -2692
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.85% <ø> (-14.72%)` | :arrow_down: |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.85% <ø> (-14.76%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.85% <ø> (-14.80%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.85% <ø> (-14.79%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.85% <ø> (-0.05%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12090/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12090?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Retrofit BaseClusterIntegrationTest to Support Multiple Pinot Clusters [pinot]
Posted by "zhtaoxiang (via GitHub)" <gi...@apache.org>.
zhtaoxiang commented on code in PR #12090:
URL: https://github.com/apache/pinot/pull/12090#discussion_r1414356699
##########
pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTest.java:
##########
@@ -158,6 +158,15 @@ protected String getKafkaZKAddress() {
return getZkUrl() + "/kafka";
}
+ /**
Review Comment:
The same for changes in this class.
##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java:
##########
@@ -123,22 +123,41 @@ public class ControllerTest {
private int _controllerPort;
private String _controllerBaseApiUrl;
+ private final Map<Integer, String> _controllerBaseApiUrls = new HashMap<>();
Review Comment:
Instead of modifying the existing `ControllerTest`, can we create another separate class `MultipleControllerTest`? This can keep the existing code easy to read
##########
pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTest.java:
##########
@@ -397,9 +406,8 @@ protected Map<String, String> getCSVDecoderProperties(@Nullable String delimiter
/**
* Creates a new Upsert enabled table config.
*/
- protected TableConfig createCSVUpsertTableConfig(String tableName, @Nullable String kafkaTopicName,
- int numPartitions, Map<String, String> streamDecoderProperties, UpsertConfig upsertConfig,
- String primaryKeyColumn) {
+ protected TableConfig createCSVUpsertTableConfig(String tableName, @Nullable String kafkaTopicName, int numPartitions,
Review Comment:
Can we please do not reformat unrelated code? reformatting makes the code harder to read.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org