You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2018/02/27 20:45:41 UTC

[GitHub] dubeejw closed pull request #20: Defect for delete of preinstalled directory

dubeejw closed pull request #20: Defect for delete of preinstalled directory
URL: https://github.com/apache/incubator-openwhisk-package-deploy/pull/20
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/packages/actions/deploy.js b/packages/actions/deploy.js
index 1c82dcc..0561cdc 100644
--- a/packages/actions/deploy.js
+++ b/packages/actions/deploy.js
@@ -1,20 +1,17 @@
 const fs = require('fs');
-const path = require('path');
-const exec = require('child_process').exec;
 const git = require('simple-git');
-const yaml = require('js-yaml');
 const common = require('./lib/common');
 
-let command = '';
 
 /**
  * Action to deploy openwhisk elements from a compliant repository
  *  @param {string} gitUrl - github url containing the manifest and elements to deploy
  *  @param {string} manifestPath - (optional) the path to the manifest file, e.g. "openwhisk/src"
- *  @param {object} envData - (optional) some specific details such as cloudant username or cloudant password
+ *  @param {object} envData - (optional) env details such as cloudant username or cloudant password
  *  @return {object} Promise
  */
 function main(params) {
+  const activationId = process.env.__OW_ACTIVATION_ID;
   return new Promise((resolve, reject) => {
     // Grab optional envData and manifestPath params for wskdeploy
     let {
@@ -25,9 +22,7 @@ function main(params) {
 
     // confirm gitUrl was provided as a parameter
     if (!gitUrl) {
-      reject({
-        error: 'Please enter the GitHub repo url in params',
-      });
+      reject(new Error('Please enter the GitHub repo url in params'));
     }
 
     // if no manifestPath was provided, use current directory
@@ -38,40 +33,50 @@ function main(params) {
     const { wskApiHost, wskAuth } = getWskApiAuth(params);
 
     // Extract the name of the repo for the tmp directory
-    const repoSplit = params.gitUrl.split('/');
-    const repoName = repoSplit[repoSplit.length - 1];
-    const localDirName = `${__dirname}/../tmp/${repoName}`;
-    return git()
-    .clone(gitUrl, localDirName, ['--depth', '1'], (err, data) => {
-      if (err) {
-        reject('There was a problem cloning from github.  Does that github repo exist?  Does it begin with http?');
-      }
+    const tmpUrl = gitUrl.replace('https://', '');
+    const repoSplit = tmpUrl.split('/');
+    const repoOrg = repoSplit[1];
+    const repoName = repoSplit[2];
+    const localDirName = `${__dirname}/../tmp/${repoOrg}/${repoName}`;
+    const templatesDirName = `${__dirname}/preInstalled/${repoOrg}/${repoName}`;
+
+    if (fs.existsSync(templatesDirName)) {
       resolve({
-        repoDir: localDirName,
+        repoDir: templatesDirName,
+        usingTemp: false,
         manifestPath,
         manifestFileName: 'manifest.yaml',
         wskAuth,
         wskApiHost,
         envData,
       });
-    });
-  })
-  .then((result) => {
-    return common.main(result);
-  })
-  .then((success) => {
-    return new Promise((resolve, reject) => {
-      resolve({
-        status: 'success',
-        success: true,
+    } else {
+      return git().clone(gitUrl, localDirName, ['--depth', '1'], (err) => {
+        if (err) {
+          reject(new Error('There was a problem cloning from github.  Does that github repo exist?  Does it begin with http?'));
+        }
+        resolve({
+          repoDir: localDirName,
+          usingTemp: true,
+          manifestPath,
+          manifestFileName: 'manifest.yaml',
+          wskAuth,
+          wskApiHost,
+          envData,
+        });
       });
-    });
-  })
-  .catch(
-    (err) => {
-      return ({error: err});
     }
-  );
+  })
+    .then(result => common.main(result))
+    .then(success =>
+      new Promise((resolve, reject) => {
+        resolve({
+          status: success,
+          activationId,
+          success: true,
+        });
+      }))
+    .catch(err => ({ error: err.message, activationId }));
 }
 
 /**
diff --git a/packages/actions/deployWeb.js b/packages/actions/deployWeb.js
index 4865549..282a9ae 100644
--- a/packages/actions/deployWeb.js
+++ b/packages/actions/deployWeb.js
@@ -1,17 +1,12 @@
 const fs = require('fs');
-const path = require('path');
-const exec = require('child_process').exec;
 const git = require('simple-git');
-const yaml = require('js-yaml');
 const common = require('./lib/common');
 
-let command = '';
-
 /**
  * Action to deploy openwhisk elements from a compliant repository
  *  @param {string} gitUrl - github url containing the manifest and elements to deploy
  *  @param {string} manifestPath - (optional) the path to the manifest file, e.g. "openwhisk/src"
- *  @param {object} envData - (optional) some specific details such as cloudant username or cloudant password
+ *  @param {object} envData - (optional) env details such as cloudant username or cloudant password
  *  @return {object} Promise
  */
 function main(params) {
@@ -25,10 +20,10 @@ function main(params) {
 
   // confirm gitUrl was provided as a parameter
   if (!gitUrl) {
-    return sendError(400, 'Please enter the GitHub repo url in params');
+    return sendError(400, new Error('Please enter the GitHub repo url in params'));
   }
 
-  if(params.__ow_method === "post") {
+  if (params.__ow_method === "post") {
     return new Promise((resolve, reject) => {
       // if no manifestPath was provided, use current directory
       if (!manifestPath) {
@@ -38,9 +33,10 @@ function main(params) {
       const { wskApiHost, wskAuth } = getWskApiAuth(params);
 
       // Extract the name of the repo for the tmp directory
-      const repoSplit = params.gitUrl.split('/');
-      const repoName = repoSplit[repoSplit.length - 1];
-      const repoOrg = repoSplit[repoSplit.length - 2];
+      const tmpUrl = gitUrl.replace('https://', '');
+      const repoSplit = tmpUrl.split('/');
+      const repoOrg = repoSplit[1];
+      const repoName = repoSplit[2];
       const localDirName = `${__dirname}/../tmp/${repoOrg}/${repoName}`;
 
       // any pre installed github repos should be a sibling to this package in "preInstalled" folder
@@ -49,21 +45,21 @@ function main(params) {
       if (fs.existsSync(templatesDirName)) {
         resolve({
           repoDir: templatesDirName,
+          usingTemp: false,
           manifestPath,
           manifestFileName: 'manifest.yaml',
           wskAuth,
           wskApiHost,
           envData,
         });
-      }
-      else {
-        return git()
-        .clone(gitUrl, localDirName, ['--depth', '1'], (err, data) => {
+      } else {
+        return git().clone(gitUrl, localDirName, ['--depth', '1'], (err) => {
           if (err) {
-            reject('There was a problem cloning from github.  Does that github repo exist?  Does it begin with http?');
+            reject(new Error('There was a problem cloning from github.  Does that github repo exist?  Does it begin with http?'));
           }
           resolve({
             repoDir: localDirName,
+            usingTemp: true,
             manifestPath,
             manifestFileName: 'manifest.yaml',
             wskAuth,
@@ -73,23 +69,16 @@ function main(params) {
         });
       }
     })
-    .then((result) => {
-      return common.main(result);
-    })
-    .then((success) => {
-      return new Promise((resolve, reject) => {
-        resolve({
-          statusCode: 200,
-          headers: {'Content-Type': 'application/json'},
-          body: new Buffer(JSON.stringify({status: success, activationId: activationId })).toString('base64')
-        });
-      });
-    })
-    .catch(
-      (err) => {
-        return (sendError(400, err));
-      }
-    );
+      .then(result => common.main(result))
+      .then(success =>
+        new Promise((resolve, reject) => {
+          resolve({
+            statusCode: 200,
+            headers: { 'Content-Type': 'application/json' },
+            body: Buffer.from(JSON.stringify({ status: success, activationId })).toString('base64'),
+          });
+        }))
+      .catch(err => (sendError(400, err)));
   }
 }
 
@@ -118,16 +107,20 @@ function getWskApiAuth(params) {
   };
 }
 
-function sendError(statusCode, error, message) {
+function sendError(statusCode, err, message) {
+  let error;
+  if (err.message) {
+    error = err.message;
+  }
   const activationId = process.env.__OW_ACTIVATION_ID;
   const params = { error, activationId };
   if (message) {
     params.message = message;
   }
   return {
-    statusCode: statusCode,
+    statusCode,
     headers: { 'Content-Type': 'application/json' },
-    body: new Buffer(JSON.stringify(params)).toString('base64')
+    body: Buffer.from(JSON.stringify(params)).toString('base64'),
   };
 }
 
diff --git a/packages/actions/lib/common.js b/packages/actions/lib/common.js
index 48f1a74..a7134f1 100644
--- a/packages/actions/lib/common.js
+++ b/packages/actions/lib/common.js
@@ -1,16 +1,11 @@
 const fs = require('fs');
 const path = require('path');
-const exec = require('child_process').exec;
-const git = require('simple-git');
-const yaml = require('js-yaml');
+const { exec } = require('child_process');
+
 let command = '';
 
 /**
- * Action to deploy openwhisk elements from a compliant repository
- *  @param {string} gitUrl - github url containing the manifest and elements to deploy
- *  @param {string} manifestPath - (optional) the path to the manifest file, e.g. "openwhisk/src"
- *  @param {object} envData - (optional) some specific details such as cloudant username or cloudant password
- *  @return {object} Promise
+ * Common function to enable deployment from deployWeb.js & deploy.js
  */
 function main(params) {
   return new Promise((resolve, reject) => {
@@ -18,6 +13,7 @@ function main(params) {
       wskAuth,
       wskApiHost,
       manifestPath,
+      usingTemp,
       manifestFileName,
       repoDir,
       envData,
@@ -40,13 +36,17 @@ function main(params) {
 
     const manifestFilePath = `${repoDir}/${manifestPath}/${manifestFileName}`;
     if (!fs.existsSync(manifestFilePath)) {
-      deleteFolder(repoDir);
-      reject(`Error loading manifest file. Does a manifest file exist?`);
+      if (usingTemp) {
+        deleteFolder(repoDir);
+      }
+      reject(new Error('Error loading manifest file. Does a manifest file exist?'));
     } else {
       exec(command, execOptions, (err, stdout, stderr) => {
-        deleteFolder(repoDir);
+        if (usingTemp) {
+          deleteFolder(repoDir);
+        }
         if (err) {
-          reject('Error running `./wskdeploy`: ', err);
+          reject(new Error('Error running `./wskdeploy`: ', err));
         }
         if (stdout) {
           console.log('stdout from wskDeploy: ', stdout, ' type ', typeof stdout);
@@ -62,7 +62,7 @@ function main(params) {
           if (typeof stdout === 'object') {
             if (stdout.error) {
               stdout.descriptiveError = 'Could not successfully run wskdeploy. Please run again with the verbose flag, -v.';
-              reject(stdout);
+              reject(new Error(stdout));
             }
           }
         }
@@ -86,12 +86,12 @@ function main(params) {
  */
 function deleteFolder(pathToDelete) {
   if (fs.existsSync(pathToDelete)) {
-    fs.readdirSync(pathToDelete).forEach(function(file, index){
-      var curPath = path.join(pathToDelete, file);
+    fs.readdirSync(pathToDelete).forEach((file, index) => {
+      const curPath = path.join(pathToDelete, file);
       if (fs.lstatSync(curPath).isDirectory()) {
         deleteFolder(curPath);
       } else {
-        //unlinkSync deletes files.
+        // unlinkSync deletes files.
         fs.unlinkSync(curPath);
       }
     });
@@ -100,5 +100,5 @@ function deleteFolder(pathToDelete) {
 }
 
 module.exports = {
-  'main': main
+  main,
 };
diff --git a/tests/src/test/scala/packages/DeployTests.scala b/tests/src/test/scala/packages/DeployTests.scala
index ae082d3..c85c3b1 100644
--- a/tests/src/test/scala/packages/DeployTests.scala
+++ b/tests/src/test/scala/packages/DeployTests.scala
@@ -57,7 +57,7 @@ class DeployTests extends TestHelpers
           activation =>
           activation.response.success shouldBe true
           val logs = activation.logs.get.toString
-          logs should include(s"Action $helloWorldAction has been successfully deployed.")
+          logs should include(s"action [$helloWorldAction] has been successfully deployed.")
         }
         // clean up after test
         wsk.action.delete(helloWorldAction)
@@ -73,7 +73,7 @@ class DeployTests extends TestHelpers
           activation =>
           activation.response.success shouldBe true
           val logs = activation.logs.get.toString
-          logs should include(s"Action $helloWorldActionPackage has been successfully deployed.")
+          logs should include(s"action [$helloWorldActionPackage] has been successfully deployed.")
         }
         // clean up after test
         wsk.action.delete(helloWorldActionPackage)
@@ -112,7 +112,7 @@ class DeployTests extends TestHelpers
           activation =>
           activation.response.success shouldBe true
           val logs = activation.logs.get.toString
-          logs should include(s"Action $helloWorldAction has been successfully deployed.")
+          logs should include(s"action [$helloWorldAction] has been successfully deployed.")
         }
         // clean up after test
         wsk.action.delete(helloWorldAction)


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services