You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by "hhcs9527 (via GitHub)" <gi...@apache.org> on 2023/03/05 13:56:44 UTC
[GitHub] [submarine] hhcs9527 opened a new pull request, #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
hhcs9527 opened a new pull request, #1020:
URL: https://github.com/apache/submarine/pull/1020
### What is this PR for?
1. Fix the syntax error reported in sonarcloud
2. Add init value for apiversion in XGboostjobList.java
3. change the following class to a singleton, [ModelVersionService.java](https://github.com/apache/submarine/pull/1020/files#diff-0b498de1dff04b42428e969cc3e0fb1376b75b9f33db4ae8d9740c7e1a2e43be), [RegisteredModelService.java](https://github.com/apache/submarine/pull/1020/files#diff-5894fdc7eea50e5065c43846308e16847562089db57b207ea23ceef86c639668),[RegisteredModelTagService.java](https://github.com/apache/submarine/pull/1020/files#diff-56494d2d82488701eb8e3738e8ff3835a27e7f5add8c9d3421ee35e4e2b09661), and the managers used thems.
### What type of PR is it?
Bug Fix
### Todos
* [ ] - Task
### What is the Jira issue?
[SUBMARINE-1349](https://issues.apache.org/jira/browse/SUBMARINE-1349)
### How should this be tested?
<!--
* First time? Setup Travis CI as described on https://submarine.apache.org/contribution/contributions.html#continuous-integration
* Strongly recommended: add automated unit tests for any new or changed behavior
* Outline any manual steps to test the PR here.
-->
### Screenshots (if appropriate)
### Questions:
* Do the license files need updating? No
* Are there breaking changes for older versions? No
* Does this need new documentation? No
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] hhcs9527 commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
hhcs9527 commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1037210828
##########
submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/xgboostjob/XGBoostJobList.java:
##########
@@ -27,7 +27,7 @@
public class XGBoostJobList implements KubernetesListObject {
@SerializedName("apiVersion")
- private String apiVersion;
+ private String apiVersion = XGBoostJob.CRD_XGBOOST_API_VERSION_V1;
@SerializedName("kind")
private String kind;
Review Comment:
@cdmikechen kind doesn't show the problem in sonarcloud.
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] cdmikechen commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
cdmikechen commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1029935069
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/manager/ModelVersionManager.java:
##########
@@ -50,14 +50,10 @@ public class ModelVersionManager {
*
* @return object
*/
- public static ModelVersionManager getInstance() {
+ public static synchronized ModelVersionManager getInstance() {
Review Comment:
We should not change the position of `synchronized`, changing it on this will cause the whole method to be locked at the time of the call.
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/manager/RegisteredModelManager.java:
##########
@@ -54,14 +54,10 @@ public class RegisteredModelManager {
*
* @return object
*/
- public static RegisteredModelManager getInstance() {
+ public static synchronized RegisteredModelManager getInstance() {
Review Comment:
Same as above
##########
submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/xgboostjob/XGBoostJobList.java:
##########
@@ -27,7 +27,7 @@
public class XGBoostJobList implements KubernetesListObject {
@SerializedName("apiVersion")
- private String apiVersion;
+ private String apiVersion = XGBoostJob.CRD_XGBOOST_API_VERSION_V1;
@SerializedName("kind")
private String kind;
Review Comment:
Does `kind` have the same problem?
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] hhcs9527 commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
hhcs9527 commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1054401052
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/s3/Client.java:
##########
@@ -45,30 +47,57 @@
/**
* S3(Minio) default client
*/
-public class Client {
+public enum Client {
+ DEFAULT(SubmarineConfiguration.getInstance().getString(SubmarineConfVars.ConfVars.SUBMARINE_S3_ENDPOINT)),
Review Comment:
Hi @cdmikechen ,
Not sure what you mean here as well. Are you suggesting not initiating the enum variable?
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] hhcs9527 commented on pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
hhcs9527 commented on PR #1020:
URL: https://github.com/apache/submarine/pull/1020#issuecomment-1345201424
@cdmikechen just updated with some modifications, please take a look. (singleton, and their reference)
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] hhcs9527 closed pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by "hhcs9527 (via GitHub)" <gi...@apache.org>.
hhcs9527 closed pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
URL: https://github.com/apache/submarine/pull/1020
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] codecov[bot] commented on pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #1020:
URL: https://github.com/apache/submarine/pull/1020#issuecomment-1323213011
# [Codecov](https://codecov.io/gh/apache/submarine/pull/1020?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1020](https://codecov.io/gh/apache/submarine/pull/1020?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f940d14) into [master](https://codecov.io/gh/apache/submarine/commit/f492e42a54331e4bccab029422bee68bc7c9ec58?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f492e42) will **increase** coverage by `26.71%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #1020 +/- ##
=============================================
+ Coverage 25.48% 52.19% +26.71%
=============================================
Files 358 112 -246
Lines 29089 4945 -24144
Branches 3479 0 -3479
=============================================
- Hits 7412 2581 -4831
+ Misses 21461 2364 -19097
+ Partials 216 0 -216
```
| Flag | Coverage Δ | |
|---|---|---|
| python-integration | `?` | |
| python-unit | `52.19% <ø> (ø)` | |
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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/submarine/pull/1020?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ine-sdk/pysubmarine/submarine/models/tensorflow.py](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VibWFyaW5lLXNkay9weXN1Ym1hcmluZS9zdWJtYXJpbmUvbW9kZWxzL3RlbnNvcmZsb3cucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...submarine/store/model\_registry/sqlalchemy\_store.py](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VibWFyaW5lLXNkay9weXN1Ym1hcmluZS9zdWJtYXJpbmUvc3RvcmUvbW9kZWxfcmVnaXN0cnkvc3FsYWxjaGVteV9zdG9yZS5weQ==) | `21.57% <0.00%> (-75.52%)` | :arrow_down: |
| [...k/pysubmarine/submarine/client/api/notebook\_api.py](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VibWFyaW5lLXNkay9weXN1Ym1hcmluZS9zdWJtYXJpbmUvY2xpZW50L2FwaS9ub3RlYm9va19hcGkucHk=) | `12.97% <0.00%> (-66.42%)` | :arrow_down: |
| [...ysubmarine/submarine/client/api/environment\_api.py](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VibWFyaW5lLXNkay9weXN1Ym1hcmluZS9zdWJtYXJpbmUvY2xpZW50L2FwaS9lbnZpcm9ubWVudF9hcGkucHk=) | `12.31% <0.00%> (-62.32%)` | :arrow_down: |
| [...arine-sdk/pysubmarine/submarine/tracking/client.py](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VibWFyaW5lLXNkay9weXN1Ym1hcmluZS9zdWJtYXJpbmUvdHJhY2tpbmcvY2xpZW50LnB5) | `30.37% <0.00%> (-55.70%)` | :arrow_down: |
| [...-sdk/pysubmarine/submarine/cli/notebook/command.py](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VibWFyaW5lLXNkay9weXN1Ym1hcmluZS9zdWJtYXJpbmUvY2xpL25vdGVib29rL2NvbW1hbmQucHk=) | `26.19% <0.00%> (-53.58%)` | :arrow_down: |
| [...k/pysubmarine/submarine/cli/environment/command.py](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VibWFyaW5lLXNkay9weXN1Ym1hcmluZS9zdWJtYXJpbmUvY2xpL2Vudmlyb25tZW50L2NvbW1hbmQucHk=) | `26.19% <0.00%> (-53.58%)` | :arrow_down: |
| [...dk/pysubmarine/submarine/cli/experiment/command.py](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VibWFyaW5lLXNkay9weXN1Ym1hcmluZS9zdWJtYXJpbmUvY2xpL2V4cGVyaW1lbnQvY29tbWFuZC5weQ==) | `25.55% <0.00%> (-52.23%)` | :arrow_down: |
| [...ine-sdk/pysubmarine/submarine/client/api\_client.py](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VibWFyaW5lLXNkay9weXN1Ym1hcmluZS9zdWJtYXJpbmUvY2xpZW50L2FwaV9jbGllbnQucHk=) | `20.14% <0.00%> (-42.41%)` | :arrow_down: |
| [...pysubmarine/submarine/client/api/experiment\_api.py](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VibWFyaW5lLXNkay9weXN1Ym1hcmluZS9zdWJtYXJpbmUvY2xpZW50L2FwaS9leHBlcmltZW50X2FwaS5weQ==) | `10.34% <0.00%> (-41.38%)` | :arrow_down: |
| ... and [269 more](https://codecov.io/gh/apache/submarine/pull/1020/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
: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=The+Apache+Software+Foundation)
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] cdmikechen commented on pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
cdmikechen commented on PR #1020:
URL: https://github.com/apache/submarine/pull/1020#issuecomment-1345274430
@hhcs9527
There are some check style and build task errors, please take care of this part first, thanks~
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] cdmikechen commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
cdmikechen commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1045175230
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/s3/Client.java:
##########
@@ -43,19 +45,38 @@
public class Client {
public MinioClient minioClient;
+ public static Map<String,MinioClient> minioClientFactory = new HashMap<String, MinioClient>();
+ public static Map<String,Client> clientFactory = new HashMap<String, Client>();
+
+ public static Client getClient(String endpoint) {
Review Comment:
Java enum class can be used inside a class to create a constructor with parameters and as a singleton. Variables are only one of its basic uses.
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] cdmikechen commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
cdmikechen commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1053868260
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/s3/Client.java:
##########
@@ -45,30 +47,57 @@
/**
* S3(Minio) default client
*/
-public class Client {
+public enum Client {
+ DEFAULT(SubmarineConfiguration.getInstance().getString(SubmarineConfVars.ConfVars.SUBMARINE_S3_ENDPOINT)),
Review Comment:
I don't think it's necessary to add an endpoint attribute to the enum, which can't declare a variable number of endpoints and has to use the internal map to handle it.
```java
public enum Client {
INSTANCE;
public static Client getInstance(String endpoint) {
// ....
}
}
Client.INSTANCE.getInstance("http://localhost:9000");
```
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/manager/ModelVersionManager.java:
##########
@@ -50,16 +50,14 @@ public class ModelVersionManager {
*
* @return object
*/
+ private static class ModelVersionManagerHolder {
+ private static ModelVersionManager manager = new ModelVersionManager(ModelVersionService.getInstance(),
+ new ModelVersionTagService(),
Review Comment:
new class param should also be avoided here.
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] hhcs9527 commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
hhcs9527 commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1045165875
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/s3/Client.java:
##########
@@ -43,19 +45,38 @@
public class Client {
public MinioClient minioClient;
+ public static Map<String,MinioClient> minioClientFactory = new HashMap<String, MinioClient>();
+ public static Map<String,Client> clientFactory = new HashMap<String, Client>();
+
+ public static Client getClient(String endpoint) {
Review Comment:
Hi @cdmikechen , I agree with you.
But the enum type is more suitable for the predefined class (no argument to init). In our case, we need to init different users in testing, which cannot be predefined; that's why I use double-check locking.
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] cdmikechen commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
cdmikechen commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1045092474
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/s3/Client.java:
##########
@@ -43,19 +45,38 @@
public class Client {
public MinioClient minioClient;
+ public static Map<String,MinioClient> minioClientFactory = new HashMap<String, MinioClient>();
+ public static Map<String,Client> clientFactory = new HashMap<String, Client>();
+
+ public static Client getClient(String endpoint) {
Review Comment:
Maybe you can use a `enum` class to replace inner factory class to avoid client double checks.
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] cdmikechen commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
cdmikechen commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1045175230
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/s3/Client.java:
##########
@@ -43,19 +45,38 @@
public class Client {
public MinioClient minioClient;
+ public static Map<String,MinioClient> minioClientFactory = new HashMap<String, MinioClient>();
+ public static Map<String,Client> clientFactory = new HashMap<String, Client>();
+
+ public static Client getClient(String endpoint) {
Review Comment:
Java enum class can be used inside itself to create a constructor with parameters and as a singleton. Variables are only one of its basic uses.
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] hhcs9527 commented on pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
hhcs9527 commented on PR #1020:
URL: https://github.com/apache/submarine/pull/1020#issuecomment-1357921402
@cdmikechen just updated Java enum class, please take a look
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] hhcs9527 commented on pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
hhcs9527 commented on PR #1020:
URL: https://github.com/apache/submarine/pull/1020#issuecomment-1323209821
Hi @cdmikechen , Please take a look at this PR, thanks.
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] hhcs9527 commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
hhcs9527 commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1054397344
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/manager/ModelVersionManager.java:
##########
@@ -50,16 +50,14 @@ public class ModelVersionManager {
*
* @return object
*/
+ private static class ModelVersionManagerHolder {
+ private static ModelVersionManager manager = new ModelVersionManager(ModelVersionService.getInstance(),
+ new ModelVersionTagService(),
Review Comment:
Hi @cdmikechen ,
Not sure what you mean by adding some test cases for the minio client section here.
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] cdmikechen commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
cdmikechen commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1055923503
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/s3/Client.java:
##########
@@ -45,30 +47,57 @@
/**
* S3(Minio) default client
*/
-public class Client {
+public enum Client {
+ DEFAULT(SubmarineConfiguration.getInstance().getString(SubmarineConfVars.ConfVars.SUBMARINE_S3_ENDPOINT)),
Review Comment:
When using an enum as a factory to get a single instance object, using a limited number of enum definitions does not achieve the final effect of a dynamic factory
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] hhcs9527 commented on pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by "hhcs9527 (via GitHub)" <gi...@apache.org>.
hhcs9527 commented on PR #1020:
URL: https://github.com/apache/submarine/pull/1020#issuecomment-1427074465
@cdmikechen we still facing the same error for being unable to connect to the local host.
Any good suggestion?
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] asfgit closed pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
URL: https://github.com/apache/submarine/pull/1020
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] cdmikechen commented on a diff in pull request #1020: SUBMARINE-1349. Fix the syntax error reported in sonarcloud and add init value for apiversion in XGboostjobList.java
Posted by GitBox <gi...@apache.org>.
cdmikechen commented on code in PR #1020:
URL: https://github.com/apache/submarine/pull/1020#discussion_r1055918763
##########
submarine-server/server-core/src/main/java/org/apache/submarine/server/manager/ModelVersionManager.java:
##########
@@ -50,16 +50,14 @@ public class ModelVersionManager {
*
* @return object
*/
+ private static class ModelVersionManagerHolder {
+ private static ModelVersionManager manager = new ModelVersionManager(ModelVersionService.getInstance(),
+ new ModelVersionTagService(),
Review Comment:
New code changes need to be supported by test cases, otherwise they will reduce code coverage and fail to prove code robustness.
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org