Skip to content
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

Improvements to Jenkinsfile #147

Merged
merged 11 commits into from
May 13, 2020
Merged

Improvements to Jenkinsfile #147

merged 11 commits into from
May 13, 2020

Conversation

justaddcoffee
Copy link
Collaborator

@justaddcoffee justaddcoffee commented May 11, 2020

  • wrap stages in dir('gitrepo') instead of repeatedly doing cd gitrepo
  • upload output of transform() to s3
  • fix logic in download() that prevents proper fxn in non-master branch

@justaddcoffee justaddcoffee requested a review from kltm May 11, 2020 21:29
Copy link
Contributor

@kltm kltm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some thoughts

Jenkinsfile Outdated
sh 'cd config;. venv/bin/activate; pigz merged-kg.tar'
dir('./gitrepo') {
sh '. venv/bin/activate; python3.7 run.py load'
sh '. venv/bin/activate; pigz merged-kg.tar'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need activate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run.py load needs the venv environment - I guess pigz doesn't though

Jenkinsfile Outdated
sh 'cd config;. venv/bin/activate; pigz merged-kg.nt'
dir('./gitrepo') {
sh '. venv/bin/activate; kgx transform --input-type tsv --output-type nt -o ./merged-kg.nt merged-kg.tar.gz'
sh '. venv/bin/activate; pigz merged-kg.nt'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is activate used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the run.py commands will need this, possibly kgx too. Looks like I can remove it from the pigz command though

Jenkinsfile Outdated
if (run_py_dl == 0) { // upload raw to s3
sh 's3cmd -c $S3CMD_JSON --acl-public --mime-type=plain/text --cf-invalidate put -r data/raw s3://kg-hub-public-data/'
} else { // 'run.py download' failed - let's try to download last good copy of raw/ from s3 to data/
sh 'rm -fr data/raw; mkdir -p data/raw'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might suggest:

'rm -r -f data/raw || true'
'mkdir -p data/raw || true'

or functionally similar

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

Jenkinsfile Outdated
sh 'cd config;. venv/bin/activate; python3.7 run.py load'
sh 'cd config;. venv/bin/activate; pigz merged-kg.tar'
dir('./gitrepo') {
sh '. venv/bin/activate; python3.7 run.py load'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe '&&' for these?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably a good idea

@justaddcoffee justaddcoffee changed the title Remove awkward, repeated 'cd config' cmds from Jenkinfile Improvements to Jenkinsfile May 13, 2020
@justaddcoffee justaddcoffee merged commit b630715 into master May 13, 2020
@justaddcoffee justaddcoffee deleted the simplify_jenkinsfile branch May 13, 2020 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants