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

DM-31899: Update example yamls including use of execution butler #92

Merged
merged 1 commit into from Nov 3, 2021

Conversation

kherner
Copy link
Contributor

@kherner kherner commented Oct 1, 2021

No description provided.

Copy link

@MichelleGower MichelleGower left a comment

Choose a reason for hiding this comment

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

There are at least a couple places that won't work with either w40 or w41 (e.g., outCollection being renamed to outputRun). I recommend having 0 command lines in the yaml and removing others that are the same as defaults that would practically never need to be changed. Definitely let me know if there are entries that have to be different than defaults (especially pipetask/butler commands). If you want me to read through it again after more changes, let me know.

bps/bps_ApPipe.yaml Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Show resolved Hide resolved
bps/bps_ApTemplate.yaml Show resolved Hide resolved
@mrawls mrawls changed the title Update example yamls including use of execution butler DM-31899: Update example yamls including use of execution butler Oct 20, 2021
Copy link
Collaborator

@mrawls mrawls left a comment

Choose a reason for hiding this comment

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

In the intervening time when I forgot about this review, bps submit files have gotten WAY simpler! So, please update this accordingly - the bps_defaults.yaml pulls in almost everything you need.

bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Outdated Show resolved Hide resolved
bps/bps_ApTemplate.yaml Show resolved Hide resolved
@kherner
Copy link
Contributor Author

kherner commented Oct 29, 2021

I think I've made all of the changes now.

Copy link
Collaborator

@mrawls mrawls left a comment

Choose a reason for hiding this comment

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

All of this looks self-consistent and correct now! I left a few suggestions for making it a notch better, but I don't need to review it again before you merge.

bps/bps_ApPipe.yaml Show resolved Hide resolved
bps/bps_ApTemplate.yaml Show resolved Hide resolved
bps/bps_ApPipe.yaml Show resolved Hide resolved
bps/bps_ApPipe.yaml Show resolved Hide resolved
bps/bps_ApTemplate.yaml Outdated Show resolved Hide resolved
bps/bps_ApPipe.yaml Show resolved Hide resolved
bps/bps_ApTemplate.yaml Show resolved Hide resolved
response to review

cleanup after review
@kherner kherner merged commit 919e8b3 into master Nov 3, 2021
@kherner kherner deleted the tickets/DM-31899 branch November 3, 2021 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants