You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airavata.apache.org by ya...@apache.org on 2026/03/30 12:41:44 UTC
(airavata) 09/16: cleanup: triage TODOs/FIXMEs — remove stale, resolve trivial, file issues for rest
This is an automated email from the ASF dual-hosted git repository.
yasith pushed a commit to branch worktree-cleanup+lean-core
in repository https://gitbox.apache.org/repos/asf/airavata.git
commit 1cd1a02b87dc747838b32bf889b1319cba6d196b
Author: yasithdev <ya...@icloud.com>
AuthorDate: Mon Mar 30 07:59:23 2026 -0400
cleanup: triage TODOs/FIXMEs — remove stale, resolve trivial, file issues for rest
Removed 18 stale/redundant TODO/FIXME comments (85 → 67):
- JSONUtil.isEqual: method was already implemented, remove errant TODO
- AppDeploymentUtil: remove commented-out setModuleLoadCmd + TODO
- CredentialReaderImpl/CommunityUserDAO: remove bare TODOs from empty methods
- CommunityUserDAO: remove 3x "fix typo" TODOs (column name was already correct)
- PBSEmailParser: remove old XSEDE/RabbitMQ infra reference
- AbstractTask.publishErrors: remove kafka TODO above bare e.printStackTrace()
- SshAgentAdaptor: remove "FIXME: Redundant" on live error-check code
- SharingServiceDBEventHandler: remove "FIXME: Remove if not required" from empty READ case
- OrchestratorServerHandler: remove 3x "TODO deprecate gfac" (gfac is gone)
- OrchestratorServerHandler: remove FIXME + entirely commented-out workflow enactment body
- GroovyMapBuilder: remove 2x "TODO fix this" wrapping dead LSF commented-out code
- LocalJobSubmissionTask: remove "TODO fix this" + entire dead gfac submission block
Remaining 67 are tracked as ISSUE (genuine missing features, security concerns,
performance notes, and arch decisions out of scope for this cleanup sweep).
Co-Authored-By: Claude Sonnet 4.6 <no...@anthropic.com>
---
.../org/apache/airavata/common/util/JSONUtil.java | 1 -
.../airavata/compute/task/GroovyMapBuilder.java | 16 ---------
.../compute/task/LocalJobSubmissionTask.java | 39 ----------------------
.../airavata/compute/util/AppDeploymentUtil.java | 2 --
.../airavata/compute/util/SshAgentAdaptor.java | 1 -
.../repository/CredentialReaderImpl.java | 1 -
.../credential/repository/db/CommunityUserDAO.java | 7 ++--
.../airavata/execution/monitor/PBSEmailParser.java | 1 -
.../execution/orchestrator/AbstractTask.java | 1 -
.../handler/SharingServiceDBEventHandler.java | 1 -
.../thrift/handler/OrchestratorServerHandler.java | 11 ------
11 files changed, 3 insertions(+), 78 deletions(-)
diff --git a/airavata-api/src/main/java/org/apache/airavata/common/util/JSONUtil.java b/airavata-api/src/main/java/org/apache/airavata/common/util/JSONUtil.java
index 6a3ea35a15..14d3610d21 100644
--- a/airavata-api/src/main/java/org/apache/airavata/common/util/JSONUtil.java
+++ b/airavata-api/src/main/java/org/apache/airavata/common/util/JSONUtil.java
@@ -57,7 +57,6 @@ public class JSONUtil {
}
public static boolean isEqual(JsonObject originalJsonObject, JsonObject newJsonObject) {
- // TODO - Implement this method
if (originalJsonObject == null && newJsonObject == null) {
return true;
} else if (originalJsonObject == null || newJsonObject == null) {
diff --git a/airavata-api/src/main/java/org/apache/airavata/compute/task/GroovyMapBuilder.java b/airavata-api/src/main/java/org/apache/airavata/compute/task/GroovyMapBuilder.java
index b4b9c2787c..c7fc9c6e29 100644
--- a/airavata-api/src/main/java/org/apache/airavata/compute/task/GroovyMapBuilder.java
+++ b/airavata-api/src/main/java/org/apache/airavata/compute/task/GroovyMapBuilder.java
@@ -123,13 +123,6 @@ public class GroovyMapBuilder {
if (jobSubmissionTaskModel.getWallTime() > 0) {
mapData.setMaxWallTime(maxWallTimeCalculator(jobSubmissionTaskModel.getWallTime()));
mapData.setWallTimeInSeconds(jobSubmissionTaskModel.getWallTime() * 60);
- // TODO fix this
- /*if (resourceJobManager != null) {
- if (resourceJobManager.getResourceJobManagerType().equals(ResourceJobManagerType.LSF)) {
- groovyMap.add(Script.MAX_WALL_TIME,
- GFacUtils.maxWallTimeCalculatorForLSF(jobSubmissionTaskModel.getWallTime()));
- }
- }*/
}
} catch (TException e) {
logger.error("Error while getting job submission sub task model", e);
@@ -164,15 +157,6 @@ public class GroovyMapBuilder {
if (scheduling.getWallTimeLimit() > 0 && mapData.getMaxWallTime() == null) {
mapData.setMaxWallTime(maxWallTimeCalculator(scheduling.getWallTimeLimit()));
mapData.setWallTimeInSeconds(scheduling.getWallTimeLimit() * 60);
-
- // TODO fix this
- /*
- if (resourceJobManager != null) {
- if (resourceJobManager.getResourceJobManagerType().equals(ResourceJobManagerType.LSF)) {
- mapData.setMaxWallTime(maxWallTimeCalculatorForLSF(scheduling.getWallTimeLimit()));
- }
- }
- */
}
if (scheduling.getTotalPhysicalMemory() > 0) {
mapData.setUsedMem(scheduling.getTotalPhysicalMemory());
diff --git a/airavata-api/src/main/java/org/apache/airavata/compute/task/LocalJobSubmissionTask.java b/airavata-api/src/main/java/org/apache/airavata/compute/task/LocalJobSubmissionTask.java
index 6a48639c13..e09394262f 100644
--- a/airavata-api/src/main/java/org/apache/airavata/compute/task/LocalJobSubmissionTask.java
+++ b/airavata-api/src/main/java/org/apache/airavata/compute/task/LocalJobSubmissionTask.java
@@ -44,45 +44,6 @@ public class LocalJobSubmissionTask extends JobSubmissionTask {
jobModel.setTaskId(getTaskId());
jobModel.setJobId(jobId);
- // TODO fix this
- /*File jobFile = SubmissionUtil.createJobFile(groovyMapData);
-
- if (jobFile != null && jobFile.exists()) {
- jobModel.setJobDescription(FileUtils.readFileToString(jobFile));
- saveJobModel(jobModel);
-
- AgentAdaptor adaptor = taskHelper.getAdaptorSupport().fetchAdaptor(
- getTaskContext().getGatewayId(),
- getTaskContext().getComputeResourceId(),
- getTaskContext().getJobSubmissionProtocol().name(),
- getTaskContext().getComputeResourceCredentialToken(),
- getTaskContext().getComputeResourceLoginUserName());
-
- GroovyMapData mapData = new GroovyMapBuilder(getTaskContext()).build();
- JobSubmissionOutput submissionOutput = submitBatchJob(adaptor, mapData, groovyMapData.getWorkingDirectory());
-
- JobStatus jobStatus = new JobStatus();
- jobStatus.setJobState(JobState.SUBMITTED);
- jobStatus.setReason("Successfully Submitted to " + getComputeResourceDescription().getHostName());
- jobStatus.setTimeOfStateChange(AiravataUtils.getCurrentTimestamp().getTime());
- jobModel.setJobStatuses(Arrays.asList(jobStatus));
-
- saveAndPublishJobStatus(jobModel);
-
- jobModel.setExitCode(submissionOutput.getExitCode());
- jobModel.setStdErr(submissionOutput.getStdErr());
- jobModel.setStdOut(submissionOutput.getStdOut());
-
- jobStatus.setJobState(JobState.COMPLETE);
- jobStatus.setReason("Successfully Completed " + getComputeResourceDescription().getHostName());
- jobStatus.setTimeOfStateChange(AiravataUtils.getCurrentTimestamp().getTime());
- jobModel.setJobStatuses(Arrays.asList(jobStatus));
-
- saveAndPublishJobStatus(jobModel);
-
- return null;
- }*/
-
return null;
} catch (Exception e) {
return null;
diff --git a/airavata-api/src/main/java/org/apache/airavata/compute/util/AppDeploymentUtil.java b/airavata-api/src/main/java/org/apache/airavata/compute/util/AppDeploymentUtil.java
index 0fa8cf9490..21db76bd9d 100644
--- a/airavata-api/src/main/java/org/apache/airavata/compute/util/AppDeploymentUtil.java
+++ b/airavata-api/src/main/java/org/apache/airavata/compute/util/AppDeploymentUtil.java
@@ -34,8 +34,6 @@ public class AppDeploymentUtil {
description.setComputeHostId(computeHost);
description.setExecutablePath(executablePath);
description.setAppDeploymentDescription(appDepDescription);
- // TODO
- // description.setModuleLoadCmd(moduleLoadCmd);
return description;
}
diff --git a/airavata-api/src/main/java/org/apache/airavata/compute/util/SshAgentAdaptor.java b/airavata-api/src/main/java/org/apache/airavata/compute/util/SshAgentAdaptor.java
index 47154d9a0b..7984a027d4 100644
--- a/airavata-api/src/main/java/org/apache/airavata/compute/util/SshAgentAdaptor.java
+++ b/airavata-api/src/main/java/org/apache/airavata/compute/util/SshAgentAdaptor.java
@@ -696,7 +696,6 @@ public class SshAgentAdaptor implements AgentAdaptor {
c = in.read();
sb.append((char) c);
} while (c != '\n');
- // FIXME: Redundant
if (b == 1) { // error
System.out.print(sb.toString());
}
diff --git a/airavata-api/src/main/java/org/apache/airavata/credential/repository/CredentialReaderImpl.java b/airavata-api/src/main/java/org/apache/airavata/credential/repository/CredentialReaderImpl.java
index 6b13860b2f..3fded1754d 100644
--- a/airavata-api/src/main/java/org/apache/airavata/credential/repository/CredentialReaderImpl.java
+++ b/airavata-api/src/main/java/org/apache/airavata/credential/repository/CredentialReaderImpl.java
@@ -158,7 +158,6 @@ public class CredentialReaderImpl implements CredentialReader, Serializable {
public void updateCommunityUserEmail(String gatewayName, String communityUser, String email)
throws CredentialStoreException {
- // TODO
}
public void removeCredentials(String gatewayName, String tokenId) throws CredentialStoreException {
diff --git a/airavata-api/src/main/java/org/apache/airavata/credential/repository/db/CommunityUserDAO.java b/airavata-api/src/main/java/org/apache/airavata/credential/repository/db/CommunityUserDAO.java
index e2c71f71d7..c69ddd0bd5 100644
--- a/airavata-api/src/main/java/org/apache/airavata/credential/repository/db/CommunityUserDAO.java
+++ b/airavata-api/src/main/java/org/apache/airavata/credential/repository/db/CommunityUserDAO.java
@@ -135,7 +135,6 @@ public class CommunityUserDAO extends ParentDAO {
public void updateCommunityUser(CommunityUser user) throws CredentialStoreException {
- // TODO
}
public CommunityUser getCommunityUser(String gatewayName, String communityUserName, Connection connection)
@@ -154,7 +153,7 @@ public class CommunityUserDAO extends ParentDAO {
ResultSet resultSet = preparedStatement.executeQuery();
if (resultSet.next()) {
- String email = resultSet.getString("COMMUNITY_USER_EMAIL"); // TODO fix typo
+ String email = resultSet.getString("COMMUNITY_USER_EMAIL");
return new CommunityUser(gatewayName, communityUserName, email);
}
@@ -191,7 +190,7 @@ public class CommunityUserDAO extends ParentDAO {
if (resultSet.next()) {
String communityUserName = resultSet.getString("COMMUNITY_USER_NAME");
- String email = resultSet.getString("COMMUNITY_USER_EMAIL"); // TODO fix typo
+ String email = resultSet.getString("COMMUNITY_USER_EMAIL");
return new CommunityUser(gatewayName, communityUserName, email);
}
@@ -229,7 +228,7 @@ public class CommunityUserDAO extends ParentDAO {
while (resultSet.next()) {
String userName = resultSet.getString("COMMUNITY_USER_NAME");
- String email = resultSet.getString("COMMUNITY_USER_EMAIL"); // TODO fix typo
+ String email = resultSet.getString("COMMUNITY_USER_EMAIL");
userList.add(new CommunityUser(gatewayName, userName, email));
}
diff --git a/airavata-api/src/main/java/org/apache/airavata/execution/monitor/PBSEmailParser.java b/airavata-api/src/main/java/org/apache/airavata/execution/monitor/PBSEmailParser.java
index 6a26bda111..4c5c7bbbd6 100644
--- a/airavata-api/src/main/java/org/apache/airavata/execution/monitor/PBSEmailParser.java
+++ b/airavata-api/src/main/java/org/apache/airavata/execution/monitor/PBSEmailParser.java
@@ -76,7 +76,6 @@ public class PBSEmailParser implements EmailParser {
case EXECUTION_TERMINATED:
int exitStatus = getExitStatus(content);
if (exitStatus == 0) {
- // TODO - Remove rabbitmq client script line from the script.
return JobState.COMPLETE;
} else if (exitStatus == 271) {
return JobState.CANCELED;
diff --git a/airavata-api/src/main/java/org/apache/airavata/execution/orchestrator/AbstractTask.java b/airavata-api/src/main/java/org/apache/airavata/execution/orchestrator/AbstractTask.java
index de3b6aeb7f..b05b7fdfd5 100644
--- a/airavata-api/src/main/java/org/apache/airavata/execution/orchestrator/AbstractTask.java
+++ b/airavata-api/src/main/java/org/apache/airavata/execution/orchestrator/AbstractTask.java
@@ -145,7 +145,6 @@ public abstract class AbstractTask extends UserContentStore implements Task {
}
protected void publishErrors(Throwable e) {
- // TODO Publish through kafka channel with task and workflow id
e.printStackTrace();
}
diff --git a/airavata-api/src/main/java/org/apache/airavata/sharing/handler/SharingServiceDBEventHandler.java b/airavata-api/src/main/java/org/apache/airavata/sharing/handler/SharingServiceDBEventHandler.java
index afcca2abf1..49883ae3a1 100644
--- a/airavata-api/src/main/java/org/apache/airavata/sharing/handler/SharingServiceDBEventHandler.java
+++ b/airavata-api/src/main/java/org/apache/airavata/sharing/handler/SharingServiceDBEventHandler.java
@@ -107,7 +107,6 @@ public class SharingServiceDBEventHandler implements MessageHandler {
break;
case READ:
- // FIXME: Remove if not required
break;
case DELETE:
diff --git a/airavata-server/thrift/src/main/java/org/apache/airavata/server/thrift/handler/OrchestratorServerHandler.java b/airavata-server/thrift/src/main/java/org/apache/airavata/server/thrift/handler/OrchestratorServerHandler.java
index c9aee2d8b7..29def22402 100644
--- a/airavata-server/thrift/src/main/java/org/apache/airavata/server/thrift/handler/OrchestratorServerHandler.java
+++ b/airavata-server/thrift/src/main/java/org/apache/airavata/server/thrift/handler/OrchestratorServerHandler.java
@@ -142,7 +142,6 @@ public class OrchestratorServerHandler implements OrchestratorService.Iface {
ExperimentModel experiment = null;
final RegistryService.Iface registryClient = getRegistryServiceClient();
try {
- // TODO deprecate this approach as we are replacing gfac
String experimentNodePath = getExperimentNodePath(experimentId);
ZKPaths.mkdirs(curatorClient.getZookeeperClient().getZooKeeper(), experimentNodePath);
String experimentCancelNode =
@@ -629,7 +628,6 @@ public class OrchestratorServerHandler implements OrchestratorService.Iface {
}
orchestrator.cancelExperiment(experimentModel, token);
- // TODO deprecate this approach as we are replacing gfac
String expCancelNodePath = ZKPaths.makePath(
ZKPaths.makePath(ZkConstants.ZOOKEEPER_EXPERIMENT_NODE, experimentId),
ZkConstants.ZOOKEEPER_CANCEL_LISTENER_NODE);
@@ -653,14 +651,6 @@ public class OrchestratorServerHandler implements OrchestratorService.Iface {
private void launchWorkflowExperiment(String experimentId, String airavataCredStoreToken, String gatewayId)
throws TException {
- // FIXME
- // try {
- // WorkflowEnactmentService.getInstance().
- // submitWorkflow(experimentId, airavataCredStoreToken, getGatewayName(),
- // getRabbitMQProcessPublisher());
- // } catch (Exception e) {
- // log.error("Error while launching workflow", e);
- // }
}
private class SingleAppExperimentRunner implements Runnable {
@@ -1020,7 +1010,6 @@ public class OrchestratorServerHandler implements OrchestratorService.Iface {
private void launchQueuedExperiment(String experimentId) throws TException, Exception {
ExperimentModel experiment = null;
final RegistryService.Iface registryClient = getRegistryServiceClient();
- // TODO deprecate this approach as we are replacing gfac
experiment = registryClient.getExperiment(experimentId);
if (experiment == null) {
throw new Exception("Error retrieving the Experiment by the given experimentID: " + experimentId);