-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added SimpleBuildReportDeploy script to deploy the build output to target libraries #149
Conversation
target libraries Signed-off-by: Anuprakash Moothedath <anuprakash.moothedath@ibm.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Anup, thanks for re-working the SimplePackageDeploy sample script. I left a couple of comments. Maybe we can meet this week to discuss them?
def buildReportFlag = 0 | ||
|
||
// Find the BuildReport.json file for deployment | ||
listDirFiles.eachFile { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than searching for the BuildReport.json in the temp directory, I think we could just assume it to be on the root level for now - this is how the packaging is performed and you make the reference to it.
The logic around a cumulative package of multiple build reports is based on the buildReportOrder.txt
, but this is fairly complex, while contents of the build reports can overlap. For the first release of this sample script, I would propose, if the buildReportOrder file is found, the script should stop processing and write out to the log that it is not supporting this packaging layout. Further on that, to me this is rather an enhancement of the packaging script to build a cumulative build report as part of its processing, than rebuilding a cumulative process here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the script and readme file for single BuildReport.json as discussed.
} | ||
|
||
if (tarExtractDir.exists()) tarExtractDir.deleteDir() | ||
println("\nDeleted the temporary folder - ${tarExtractDirName}\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather phrase that as Cleaning up the temporary folder
. But these are just words
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reworded as advised.
Signed-off-by: Anuprakash Moothedath <anuprakash.moothedath@ibm.com>
Signed-off-by: Anuprakash Moothedath <anuprakash.moothedath@ibm.com>
Signed-off-by: Anuprakash Moothedath <anuprakash.moothedath@ibm.com>
Signed-off-by: Anuprakash Moothedath <anuprakash.moothedath@ibm.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anuprakash-Moothedath Thanks for integrating the comments we discussed earlier.
The script works nicely. My new comments are targeting the wording and formatting of the outputs to increase readability.
|
||
|
||
// Untar the tar file | ||
println("** Untar file at $tarFile.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was running your script, and this output in the log confused me.
** Untar file at /u/dbehm/test-zapp-app-out/mortgageout/build.20221111.022101.021/build.20221111.022101.021.tar.
Package untar done to /u/dbehm/git/anup/work/DeployFiles_20221111.025913.059
I would rather rephrase this to
** Untarring file /u/dbehm/test-zapp-app-out/mortgageout/build.20221111.022101.021/build.20221111.022101.021.tar. to /u/dbehm/git/anup/work/DeployFiles_20221111.025913.059
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
} | ||
|
||
if (tarExtractDir.exists()) tarExtractDir.deleteDir() | ||
println("\nCleaning up the temporary folder - ${tarExtractDirName}\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please prefix with the **
to have a better reading of the output log. Also, I don't think we need the newline at the beginning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
copy.setMember("$member"); | ||
copy.copy() | ||
|
||
println("Copied source file - $dataset/$member to Target PDS - $targetPDS") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's indent the outputs a bit for better reading of the outputs:
Also, I would like to suggest to rephrase the output as:
Copied file xxx to target library yyy using DBB CopyMode zzz
** Deploying the contents in /u/dbehm/git/anup/work/DeployFiles_20221111.025913.059/BuildReport.json
Extracted file /u/dbehm/git/anup/work/DeployFiles_20221111.025913.059/DBEHM.DBB.BUILD.DBRM/EPSCMORT is of type DBRM
Copied source file - DBEHM.DBB.BUILD.DBRM/EPSCMORT to Target PDS - DBEHM.DBB.BUILD.MORTGAGE.DBRM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
} | ||
|
||
def targetPDS = "${props.hlq}" + "." + targetLibLLQMap["${it.deployType}"] | ||
println("\nExtracted file $srcFilePath is of type ${it.deployType}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that we need this extra output.
The next output line should be sufficient by the reader.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted extra output.
|
||
println("Copied source file - $dataset/$member to Target PDS - $targetPDS") | ||
} else { | ||
println("ERROR: DEPLOYMENT FAILED") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation of text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
println("ERROR: DBB COPY MODE NOT DEFINED FOR DEPLOY TYPE : ${it.deployType}\n") | ||
|
||
if (tarExtractDir.exists()) tarExtractDir.deleteDir() | ||
println("\nCleaning up the temporary folder - ${tarExtractDirName}\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, this is a candidate for prefix **
again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Signed-off-by: Anuprakash Moothedath <anuprakash.moothedath@ibm.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anuprakash-Moothedath Thanks for the contribution and modifications. This looks very good.
Please see some very tiny change requests.
deployFromBuildReport(buildReportFile,tarExtractDirName,targetLibLLQMap,copyModeMap) | ||
} else { | ||
println("** Build report data at $tarExtractDirName/BuildReport.json not found") | ||
println("** Deployment stopped") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, we should also need exit the script, so the pipeline step can be flagged as failed.
System.exit(1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, Also put eye-catcher *! for the error message.
|
||
def srcFile = new File("$srcFilePath") | ||
if(!srcFile.exists()){ | ||
println("** Source file not found at $srcFilePath") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use *!
as an eye-catcher
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Signed-off-by: Anuprakash Moothedath <anuprakash.moothedath@ibm.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates!
target libraries
Signed-off-by: Anuprakash Moothedath anuprakash.moothedath@ibm.com