[GitHub] [airavata-mft] DImuthuUpe commented on a diff in pull request #99: Transport config add ons

["DImuthuUpe (via GitHub)" <gi...@apache.org>]

DImuthuUpe commented on code in PR #99:
URL: https://github.com/apache/airavata-mft/pull/99#discussion_r1161107474


##########
transport/local-transport/src/main/java/org/apache/airavata/mft/transport/local/LocalIncomingChunkedConnector.java:
##########
@@ -58,36 +60,37 @@ public void downloadChunk(int chunkId, long startByte, long endByte, String down
         logger.info("Downloading chunk {} with start byte {} and end byte {} to file {} from resource path {}",
                 chunkId, startByte, endByte, downloadFile, this.resourcePath);
 
-//        #use this code on a DMA enabled device
-//        if (resourceSize <= endByte - startByte) {
-//            Files.copy(Path.of(this.resourcePath), Path.of(downloadFile));
-//        } else {
-//            try (FileInputStream from = new FileInputStream(this.resourcePath);
-//                 FileOutputStream to = new FileOutputStream(downloadFile)) {
-//                from.getChannel().transferTo(startByte, endByte - startByte, to.getChannel());
-//            } catch (Exception e) {
-//                logger.error("Unexpected error occurred while downloading chunk {} to file {} from resource path {}",
-//                        chunkId, downloadFile, this.resourcePath, e);
-//                throw e;
-//            }
-//        }
-
-        int buffLen = 1024 * 1024 * 16;
-        try (BufferedInputStream bis = new BufferedInputStream(new FileInputStream(this.resourcePath),buffLen);
-             BufferedOutputStream bos = new BufferedOutputStream(new FileOutputStream(downloadFile))) {
-            byte[] buffer = new byte[buffLen];
-            int read = 0;
-            long totalRead = bis.skip(startByte);
-            while ((read = bis.read(buffer,0,Math.min(buffLen, (int) (endByte - totalRead )))) > 0) {
-                bos.write(buffer, 0, read);
-                totalRead += read;
+        if (this.dmaFlag) {
+            if (resourceSize <= endByte - startByte) {
+                Files.copy(Path.of(this.resourcePath), Path.of(downloadFile));
+            } else {
+                try (FileInputStream from = new FileInputStream(this.resourcePath);
+                     FileOutputStream to = new FileOutputStream(downloadFile)) {
+                    from.getChannel().transferTo(startByte, endByte - startByte, to.getChannel());
+                } catch (Exception e) {
+                    logger.error("Unexpected error occurred while downloading chunk {} to file {} from resource path {}",
+                            chunkId, downloadFile, this.resourcePath, e);
+                    throw e;
+                }
             }
-            bis.close();
-            bos.close();
-        } catch (Exception e) {
-            logger.error("Unexpected error occurred while downloading chunk {} to file {} from resource path {}",
+        }   else {
+            int buffLen = 1024 * 1024 * 16;

Review Comment:
   We can externalize the buffer length as well



##########
agent/service/src/main/java/org/apache/airavata/mft/agent/TransportConfig.java:
##########
@@ -0,0 +1,25 @@
+package org.apache.airavata.mft.agent;

Review Comment:
   Add license headers



##########
transport/local-transport/src/main/java/org/apache/airavata/mft/transport/local/LocalIncomingChunkedConnector.java:
##########
@@ -32,13 +32,15 @@ public class LocalIncomingChunkedConnector implements IncomingChunkedConnector {
 
     private String resourcePath;
     private long resourceSize;
+    private boolean dmaFlag;
 
     private static final Logger logger = LoggerFactory.getLogger(LocalIncomingChunkedConnector.class);
 
     @Override
     public void init(ConnectorConfig connectorConfig) throws Exception {
         this.resourcePath = connectorConfig.getResourcePath();
         this.resourceSize = connectorConfig.getMetadata().getFile().getResourceSize();
+        this.dmaFlag = connectorConfig.getTransportConfig().get("local.dma").toString().equals("true");

Review Comment:
   It is better if there is a utility class to parse these configs. It will enable proper error handling and automatic type parsing
   
   Booelan dma = TransportProperties.getBoolean(connectorConfig.getTransportConfig(), "local.dma", True) // Here we parse default value if the property is not available in the transport config



-- 
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: issues-unsubscribe@airavata.apache.org

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