ofuks commented on a change in pull request #842:
URL: https://github.com/apache/incubator-dlab/pull/842#discussion_r465700704
##########
File path:
services/self-service/src/main/java/com/epam/dlab/backendapi/service/impl/ComputationalServiceImpl.java
##########
@@ -297,7 +297,7 @@ public void startSparkCluster(@User UserInfo userInfo,
String expName, @Resource
@Audit(action = RECONFIGURE, type = COMPUTE)
@Override
- public void updateSparkClusterConfig(@User UserInfo userInfo, @Project
String project, String exploratoryName, @ResourceName String computationalName,
List<ClusterConfig> config) {
+ public void updateSparkClusterConfig(@User UserInfo userInfo, @Project
String project, String exploratoryName, @ResourceName String computationalName,
List<ClusterConfig> config, @Info String auditMessage) {
Review comment:
The same here
##########
File path:
services/self-service/src/test/java/com/epam/dlab/backendapi/service/impl/ComputationalServiceImplTest.java
##########
@@ -667,7 +668,8 @@ public void
testUpdateSparkClusterConfigWhenClusterIsNotFound() {
when(exploratoryDAO.fetchExploratoryFields(anyString(), anyString(),
anyString(), anyBoolean())).thenReturn(userInstanceDto);
try {
computationalService.updateSparkClusterConfig(getUserInfo(),
PROJECT, EXPLORATORY_NAME,
- COMP_NAME + "X", config);
+ COMP_NAME + "X", config,
+ String.format(COMPUTATIONAL_RECONFIGURE_MESSAGE,
COMP_NAME, NOTE_BOOK_NAME));
Review comment:
Can we inline this?
##########
File path:
services/self-service/src/main/java/com/epam/dlab/backendapi/service/ComputationalService.java
##########
@@ -65,7 +65,7 @@ boolean createDataEngineService(UserInfo userInfo, String
resourceName, Computat
void startSparkCluster(UserInfo userInfo, String exploratoryName, String
computationalName, String project, String auditInfo);
void updateSparkClusterConfig(UserInfo userInfo, String project, String
exploratoryName, String computationalName,
- List<ClusterConfig> config);
+ List<ClusterConfig> config, String
auditMessage);
Review comment:
Please rename parameter **auditMessage** -> **auditInfo**. Because we
use auditInfo everywhere
##########
File path:
services/self-service/src/test/java/com/epam/dlab/backendapi/service/impl/ComputationalServiceImplTest.java
##########
@@ -613,13 +615,13 @@ public void testUpdateSparkClusterConfig() {
final List<ClusterConfig> config = Collections.singletonList(new
ClusterConfig());
userInstanceDto.setResources(Collections.singletonList(getUserComputationalResource(RUNNING,
COMP_NAME)));
when(exploratoryDAO.fetchExploratoryFields(anyString(), anyString(),
anyString(), anyBoolean())).thenReturn(userInstanceDto);
- when(requestBuilder.newClusterConfigUpdate(any(UserInfo.class),
any(UserInstanceDTO.class),
- any(UserComputationalResource.class),
anyListOf(ClusterConfig.class), any(EndpointDTO.class)))
- .thenReturn(clusterConfigDTO);
+ when(requestBuilder.newClusterConfigUpdate(any(UserInfo.class),
any(UserInstanceDTO.class), any(UserComputationalResource.class),
+ anyListOf(ClusterConfig.class),
+ any(EndpointDTO.class))).thenReturn(clusterConfigDTO);
Review comment:
Why did you do these changes?
##########
File path:
services/self-service/src/main/java/com/epam/dlab/backendapi/service/impl/ProjectServiceImpl.java
##########
@@ -344,7 +344,8 @@ private String getUpdateBudgetAudit(ProjectDTO p) {
Integer value =
Optional.ofNullable(get(p.getName()).getBudget())
.map(BudgetDTO::getValue)
.orElse(null);
- return String.format(AUDIT_UPDATE_BUDGET, value,
p.getBudget().getValue());
+ boolean monthlyBudget =
get(p.getName()).getBudget().isMonthlyBudget();
Review comment:
We have already made a call to Mongo in line 344. Please refactor code
do avoid a double call to DB.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
