-
Notifications
You must be signed in to change notification settings - Fork 136
Reference cleanup scripts in generateCleanupCommand.sh #307
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
base: main
Are you sure you want to change the base?
Reference cleanup scripts in generateCleanupCommand.sh #307
Conversation
Signed-off-by: Dennis Behm <[email protected]>
Co-authored-by: Lauren Li <[email protected]>
Signed-off-by: Dennis Behm <[email protected]>
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.
This PR looks good to me now - thanks @dennis-behm for working on it!
# cleanupDbbMetadataStoreOptions="--type file --location /u/github/" | ||
# for db2 metadatastore, reusing build settings | ||
# cleanupDbbMetadataStoreOptions="--type db2 --user ${dbbMetadataStoreJdbcId} --password-file ${dbbMetadataStoreJdbcPwdFile} --db2-config ${dbbMetadataStoreJdbcConfigFile} --url ${dbbMetadataStoreJdbcUrl}" | ||
cleanupDbbMetadataStoreOptions="" |
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.
We should not introduce a new variable for this cleanup, and compute the string in the generateCleanupCommands.sh
based on the information that already present in the pipelineBackend.config file (dbbMetadataStoreJdbcId, dbbMetadataStoreJdbcPwdFile, dbbMetadataStoreJdbcConfigFile and dbbMetadataStoreJdbcUrl).
By the way, there should be a dbbMetadataStoreType (file|db2) parm and a dbbMetadataStoreFileLocation parm as well in pipelineBackend.config, to help compute the right connection string.
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 @M-DLB ,
By the way, there should be a dbbMetadataStoreType (file|db2) parm and a dbbMetadataStoreFileLocation parm as well in pipelineBackend.config, to help compute the right connection string.
I understand the point here - however, I don't want to maintain two variables with the same purpose - one in the build framework and another time in the pipelineBackend.config maintaining for maintaining the dbb metadatastore type.
Could we leave it as is now? Users need to customise the pipelineBackend.config anyway, and there are clear samples provided.
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.
You already have variables for metadatstore duplicated in the build framework and in the pipelineBackend.config file. I think it would be cleaner to add the two that are missing (dbbMetadataStoreType (file|db2) parm and a dbbMetadataStoreFileLocation parm) to completeness, remove the cleanupDbbMetadataStoreOptions
parm and compute it instead.
You should aim for consistency, instead of doing one way with DB2 and another way with file metadatastore.
…etch external dependencies (IBM#299) * (CBS) packageBuildOutputs.sh to follow implemented naming conventions how archives are published to Artifact repository * (CBS) dbbBuild.sh and zBuilder.sh to process build dependency configuration defined in the Application Descriptor * (CBS) Download external dependencies based on naming conventions from the artifact repository (Nexus, Artifactory) using the ArtifactRepositoryHelpers * (CBS) Expanding the external dependencies in the build workspace for consumption by the build framework * (CBS) Central cache of external archives * (CBS) Enable wazideploy-generate to automatically download the tar from the Artifact repository * (CBS) Scripts no longer carry configuration parameters to point to existing scripts. It references the scripts directly within this repo. --------- Signed-off-by: Dennis Behm <[email protected]> Co-authored-by: Mathieu Dalbin <[email protected]> Co-authored-by: Lauren Li <[email protected]>
…script-reference-missing-in-cbs-genereatecleanupcommandssh
This addresses the defect reported in #306. It already aligns with the coming updates in #299 to reference the pipeline and utility tasks right from the same repo, keeping the pipelineBackend.config lightweight.