machristie commented on code in PR #329:
URL: https://github.com/apache/airavata/pull/329#discussion_r1015935807


##########
modules/airavata-metascheduler/process-scheduler/src/main/java/org/apache/airavata/metascheduler/process/scheduling/engine/cr/selection/DefaultComputeResourceSelectionPolicy.java:
##########
@@ -0,0 +1,105 @@
+package 
org.apache.airavata.metascheduler.process.scheduling.engine.cr.selection;
+
+import org.apache.airavata.agents.api.AgentAdaptor;
+import org.apache.airavata.agents.api.CommandOutput;
+import org.apache.airavata.helix.core.support.adaptor.AdaptorSupportImpl;
+import org.apache.airavata.metascheduler.core.adaptor.output.OutputParser;
+import 
org.apache.airavata.metascheduler.process.scheduling.engine.output.OutputParserImpl;
+import 
org.apache.airavata.model.appcatalog.computeresource.ComputeResourceDescription;
+import 
org.apache.airavata.model.appcatalog.computeresource.JobSubmissionInterface;
+import 
org.apache.airavata.model.appcatalog.computeresource.JobSubmissionProtocol;
+import 
org.apache.airavata.model.appcatalog.groupresourceprofile.GroupComputeResourcePreference;
+import org.apache.airavata.model.experiment.ExperimentModel;
+import org.apache.airavata.model.experiment.UserConfigurationDataModel;
+import org.apache.airavata.model.process.ProcessModel;
+import 
org.apache.airavata.model.scheduling.ComputationalResourceSchedulingModel;
+import org.apache.airavata.registry.api.RegistryService;
+import org.apache.airavata.registry.api.RegistryService.Client;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * This class implements selecting compute resource defined in 
USER_CONFIGURATION_DATA and assumes only one
+ * compute resource is selected for experiment.
+ * This checks whether defined CR is live
+ */
+public class DefaultComputeResourceSelectionPolicy extends 
ComputeResourceSelectionPolicyImpl {
+
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(DefaultComputeResourceSelectionPolicy.class);
+
+    @Override
+    public Optional<ComputationalResourceSchedulingModel> 
selectComputeResource(String processId) {
+        final RegistryService.Client registryClient = 
this.registryClientPool.getResource();
+        try {
+            ProcessModel processModel = registryClient.getProcess(processId);
+
+            ExperimentModel experiment = 
registryClient.getExperiment(processModel.getExperimentId());
+
+
+            UserConfigurationDataModel userConfigurationDataModel = 
experiment.getUserConfigurationData();
+
+            // Assume scheduling data is populated in 
USER_CONFIGURATION_DATA_MODEL
+            ComputationalResourceSchedulingModel 
computationalResourceSchedulingModel = userConfigurationDataModel
+                    .getComputationalResourceScheduling();
+
+            String computeResourceId = 
computationalResourceSchedulingModel.getResourceHostId();
+
+            ComputeResourceDescription comResourceDes = 
registryClient.getComputeResource(computeResourceId);
+
+            List<JobSubmissionInterface> jobSubmissionInterfaces = 
comResourceDes.getJobSubmissionInterfaces();
+            Collections.sort(jobSubmissionInterfaces, 
Comparator.comparingInt(JobSubmissionInterface::getPriorityOrder));
+            JobSubmissionProtocol jobSubmissionProtocol = 
jobSubmissionInterfaces.get(0).getJobSubmissionProtocol();
+
+            AdaptorSupportImpl adaptorSupport = 
AdaptorSupportImpl.getInstance();
+
+            String computeResourceToken = getComputeResourceCredentialToken(
+                    experiment.getGatewayId(),
+                    processModel.getUserName(),
+                    computeResourceId,
+                    processModel.isUseUserCRPref(),
+                    processModel.isSetGroupResourceProfileId(),
+                    processModel.getGroupResourceProfileId());
+
+            String loginUsername = 
getComputeResourceLoginUserName(experiment.getGatewayId(),
+                    processModel.getUserName(),
+                    computeResourceId,
+                    processModel.isUseUserCRPref(),
+                    processModel.isSetGroupResourceProfileId(),
+                    processModel.getGroupResourceProfileId(),
+                    
computationalResourceSchedulingModel.getOverrideLoginUserName());
+
+            AgentAdaptor adaptor = 
adaptorSupport.fetchAdaptor(experiment.getGatewayId(),
+                    computeResourceId,
+                    jobSubmissionProtocol,
+                    computeResourceToken,
+                    loginUsername);
+
+            GroupComputeResourcePreference computeResourcePreference = 
getGroupComputeResourcePreference(computeResourceId,
+                    processModel.getGroupResourceProfileId());
+
+
+            String command = "srun --nodes 1 --time 00:01:00 --account"

Review Comment:
   srun is SLURM specific, I believe. That's probably fine for now but 
eventually what you would want is to look up the ResourceJobManagerType for the 
JobSubmissionInterface, then call the right command for that type. See 
TaskContext.getResourceJobManager().
   
   Also, 'srun', instead of hard coded, could be another configured 
JobManagerCommand.



##########
modules/airavata-metascheduler/process-scheduler/src/main/java/org/apache/airavata/metascheduler/process/scheduling/engine/cr/selection/ComputeResourceSelectionPolicyImpl.java:
##########
@@ -0,0 +1,108 @@
+package 
org.apache.airavata.metascheduler.process.scheduling.engine.cr.selection;
+
+import org.apache.airavata.common.utils.ThriftClientPool;
+import 
org.apache.airavata.metascheduler.core.engine.ComputeResourceSelectionPolicy;
+import org.apache.airavata.metascheduler.core.utils.Utils;
+import 
org.apache.airavata.model.appcatalog.groupresourceprofile.GroupComputeResourcePreference;
+import 
org.apache.airavata.model.appcatalog.groupresourceprofile.GroupResourceProfile;
+import 
org.apache.airavata.model.appcatalog.userresourceprofile.UserComputeResourcePreference;
+import 
org.apache.airavata.model.appcatalog.userresourceprofile.UserResourceProfile;
+
+import org.apache.airavata.registry.api.RegistryService;
+import org.apache.airavata.registry.api.RegistryService.Client;
+import org.apache.airavata.registry.api.exception.RegistryServiceException;
+
+public abstract class ComputeResourceSelectionPolicyImpl implements 
ComputeResourceSelectionPolicy {
+
+    protected ThriftClientPool<RegistryService.Client> registryClientPool;
+
+    public ComputeResourceSelectionPolicyImpl() {
+        this.registryClientPool =Utils.getRegistryServiceClientPool();
+    }
+
+    private boolean isValid(String str) {
+        return str != null && !str.trim().isEmpty();
+    }
+
+    public UserResourceProfile getUserResourceProfile(String username, String 
gatewayId) throws Exception {
+        RegistryService.Client client = this.registryClientPool.getResource();
+        try {
+            return client.getUserResourceProfile(username, gatewayId);
+        }finally {
+            this.registryClientPool.returnResource(client);
+        }
+
+    }
+
+
+    private UserComputeResourcePreference 
getUserComputeResourcePreference(String gatewayId, String username,
+                                                                           
String computeResourceId) throws Exception {
+        RegistryService.Client client = this.registryClientPool.getResource();
+        try {
+            return client.getUserComputeResourcePreference(username, 
gatewayId, computeResourceId);
+        }finally {
+            this.registryClientPool.returnResource(client);
+        }
+    }
+
+    public String getComputeResourceCredentialToken(String gatewayId,
+                                                    String username, String 
computeResourceId, boolean isUseUserCRPref,
+                                                    boolean 
isSetGroupResourceProfileId, String groupResourceProfileId) throws Exception {
+        if (isUseUserCRPref) {
+            if (getUserComputeResourcePreference(gatewayId, username, 
computeResourceId) != null &&

Review Comment:
   getUserComputeResourcePreference will never return null because 
RegistryService.Client.getUserComputeResourcePreference() will never return 
null. Thrift RPC methods can't return null and instead will throw an exception. 
 In getUserComputeResourcePreference() I think you could catch the exception 
and return null, but ideally we'd have a Thrift method that does the existence 
check and returns a boolean (like 
RegistryServerHandler.isUserResourceProfileExists).



##########
modules/airavata-metascheduler/process-scheduler/src/main/java/org/apache/airavata/metascheduler/process/scheduling/engine/cr/selection/DefaultComputeResourceSelectionPolicy.java:
##########
@@ -0,0 +1,105 @@
+package 
org.apache.airavata.metascheduler.process.scheduling.engine.cr.selection;
+
+import org.apache.airavata.agents.api.AgentAdaptor;
+import org.apache.airavata.agents.api.CommandOutput;
+import org.apache.airavata.helix.core.support.adaptor.AdaptorSupportImpl;
+import org.apache.airavata.metascheduler.core.adaptor.output.OutputParser;
+import 
org.apache.airavata.metascheduler.process.scheduling.engine.output.OutputParserImpl;
+import 
org.apache.airavata.model.appcatalog.computeresource.ComputeResourceDescription;
+import 
org.apache.airavata.model.appcatalog.computeresource.JobSubmissionInterface;
+import 
org.apache.airavata.model.appcatalog.computeresource.JobSubmissionProtocol;
+import 
org.apache.airavata.model.appcatalog.groupresourceprofile.GroupComputeResourcePreference;
+import org.apache.airavata.model.experiment.ExperimentModel;
+import org.apache.airavata.model.experiment.UserConfigurationDataModel;
+import org.apache.airavata.model.process.ProcessModel;
+import 
org.apache.airavata.model.scheduling.ComputationalResourceSchedulingModel;
+import org.apache.airavata.registry.api.RegistryService;
+import org.apache.airavata.registry.api.RegistryService.Client;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * This class implements selecting compute resource defined in 
USER_CONFIGURATION_DATA and assumes only one
+ * compute resource is selected for experiment.
+ * This checks whether defined CR is live
+ */
+public class DefaultComputeResourceSelectionPolicy extends 
ComputeResourceSelectionPolicyImpl {
+
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(DefaultComputeResourceSelectionPolicy.class);
+
+    @Override
+    public Optional<ComputationalResourceSchedulingModel> 
selectComputeResource(String processId) {
+        final RegistryService.Client registryClient = 
this.registryClientPool.getResource();
+        try {
+            ProcessModel processModel = registryClient.getProcess(processId);
+
+            ExperimentModel experiment = 
registryClient.getExperiment(processModel.getExperimentId());
+
+
+            UserConfigurationDataModel userConfigurationDataModel = 
experiment.getUserConfigurationData();
+
+            // Assume scheduling data is populated in 
USER_CONFIGURATION_DATA_MODEL
+            ComputationalResourceSchedulingModel 
computationalResourceSchedulingModel = userConfigurationDataModel
+                    .getComputationalResourceScheduling();
+
+            String computeResourceId = 
computationalResourceSchedulingModel.getResourceHostId();
+
+            ComputeResourceDescription comResourceDes = 
registryClient.getComputeResource(computeResourceId);
+
+            List<JobSubmissionInterface> jobSubmissionInterfaces = 
comResourceDes.getJobSubmissionInterfaces();
+            Collections.sort(jobSubmissionInterfaces, 
Comparator.comparingInt(JobSubmissionInterface::getPriorityOrder));
+            JobSubmissionProtocol jobSubmissionProtocol = 
jobSubmissionInterfaces.get(0).getJobSubmissionProtocol();
+
+            AdaptorSupportImpl adaptorSupport = 
AdaptorSupportImpl.getInstance();
+
+            String computeResourceToken = getComputeResourceCredentialToken(
+                    experiment.getGatewayId(),
+                    processModel.getUserName(),
+                    computeResourceId,
+                    processModel.isUseUserCRPref(),
+                    processModel.isSetGroupResourceProfileId(),
+                    processModel.getGroupResourceProfileId());
+
+            String loginUsername = 
getComputeResourceLoginUserName(experiment.getGatewayId(),
+                    processModel.getUserName(),
+                    computeResourceId,
+                    processModel.isUseUserCRPref(),
+                    processModel.isSetGroupResourceProfileId(),
+                    processModel.getGroupResourceProfileId(),
+                    
computationalResourceSchedulingModel.getOverrideLoginUserName());
+
+            AgentAdaptor adaptor = 
adaptorSupport.fetchAdaptor(experiment.getGatewayId(),
+                    computeResourceId,
+                    jobSubmissionProtocol,
+                    computeResourceToken,
+                    loginUsername);
+
+            GroupComputeResourcePreference computeResourcePreference = 
getGroupComputeResourcePreference(computeResourceId,
+                    processModel.getGroupResourceProfileId());
+
+
+            String command = "srun --nodes 1 --time 00:01:00 --account"
+                    + computeResourcePreference.getAllocationProjectNumber() + 
" --partition "
+                    + computeResourcePreference.getPreferredBatchQueue() +

Review Comment:
   I'm not sure that we really use preferredBatchQueue, but also, why not use 
the computationalResourceSchedulingModel.queueName?



##########
airavata-api/airavata-data-models/src/main/java/org/apache/airavata/model/status/ProcessState.java:
##########
@@ -42,7 +42,10 @@ public enum ProcessState implements org.apache.thrift.TEnum {
   COMPLETED(10),
   FAILED(11),
   CANCELLING(12),
-  CANCELED(13);
+  CANCELED(13),

Review Comment:
   You must have updated status_models.thrift, but that file is missing from 
the PR.



##########
modules/registry/registry-core/src/main/java/org/apache/airavata/registry/core/entities/expcatalog/ComputationalResourceSchedulingEntity.java:
##########
@@ -0,0 +1,162 @@
+package org.apache.airavata.registry.core.entities.expcatalog;
+
+import javax.persistence.*;
+import java.io.Serializable;
+
+/**
+ * Persistent class for computational_resource_scheduling data table.
+ */
+@Entity
+@Table(name = "COMPUTE_RESOURCE_SCHEDULING")
+@IdClass(ComputationalResourceSchedulingPK.class)
+public class ComputationalResourceSchedulingEntity implements Serializable {

Review Comment:
   I don't think this is needed. There's a mismatch between the Thrift data 
model and the database entities here. UserConfigurationDataModel and 
ComputationalResourceSchedulingModel map to the single 
UserConfigurationDataEntity/table in the database.



##########
modules/airavata-metascheduler/process-scheduler/src/main/java/org/apache/airavata/metascheduler/process/scheduling/engine/cr/selection/ComputeResourceSelectionPolicyImpl.java:
##########
@@ -0,0 +1,108 @@
+package 
org.apache.airavata.metascheduler.process.scheduling.engine.cr.selection;
+
+import org.apache.airavata.common.utils.ThriftClientPool;
+import 
org.apache.airavata.metascheduler.core.engine.ComputeResourceSelectionPolicy;
+import org.apache.airavata.metascheduler.core.utils.Utils;
+import 
org.apache.airavata.model.appcatalog.groupresourceprofile.GroupComputeResourcePreference;
+import 
org.apache.airavata.model.appcatalog.groupresourceprofile.GroupResourceProfile;
+import 
org.apache.airavata.model.appcatalog.userresourceprofile.UserComputeResourcePreference;
+import 
org.apache.airavata.model.appcatalog.userresourceprofile.UserResourceProfile;
+
+import org.apache.airavata.registry.api.RegistryService;
+import org.apache.airavata.registry.api.RegistryService.Client;
+import org.apache.airavata.registry.api.exception.RegistryServiceException;
+
+public abstract class ComputeResourceSelectionPolicyImpl implements 
ComputeResourceSelectionPolicy {
+
+    protected ThriftClientPool<RegistryService.Client> registryClientPool;
+
+    public ComputeResourceSelectionPolicyImpl() {
+        this.registryClientPool =Utils.getRegistryServiceClientPool();
+    }
+
+    private boolean isValid(String str) {
+        return str != null && !str.trim().isEmpty();
+    }
+
+    public UserResourceProfile getUserResourceProfile(String username, String 
gatewayId) throws Exception {
+        RegistryService.Client client = this.registryClientPool.getResource();
+        try {
+            return client.getUserResourceProfile(username, gatewayId);
+        }finally {
+            this.registryClientPool.returnResource(client);
+        }
+
+    }
+
+
+    private UserComputeResourcePreference 
getUserComputeResourcePreference(String gatewayId, String username,
+                                                                           
String computeResourceId) throws Exception {
+        RegistryService.Client client = this.registryClientPool.getResource();
+        try {
+            return client.getUserComputeResourcePreference(username, 
gatewayId, computeResourceId);
+        }finally {
+            this.registryClientPool.returnResource(client);
+        }
+    }
+
+    public String getComputeResourceCredentialToken(String gatewayId,
+                                                    String username, String 
computeResourceId, boolean isUseUserCRPref,
+                                                    boolean 
isSetGroupResourceProfileId, String groupResourceProfileId) throws Exception {
+        if (isUseUserCRPref) {
+            if (getUserComputeResourcePreference(gatewayId, username, 
computeResourceId) != null &&

Review Comment:
   likewise for 
   - getUserResourceProfile
   - getGroupComputeResourcePreference
   - getGroupResourceProfile



-- 
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: dev-unsubscr...@airavata.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to