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-38142: Update ctrl_bps_panda/config/bps_usdf.yaml to allow for local custom … #40

Merged
merged 1 commit into from Mar 1, 2023

Conversation

zhaoyuyoung
Copy link
Contributor

…setup

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

@codecov
Copy link

codecov bot commented Feb 28, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (eaf9ea1) 28.66% compared to head (f8afffe) 28.66%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #40   +/-   ##
=======================================
  Coverage   28.66%   28.66%           
=======================================
  Files          10       10           
  Lines         457      457           
  Branches       81       81           
=======================================
  Hits          131      131           
  Misses        323      323           
  Partials        3        3           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@zhaoyuyoung zhaoyuyoung force-pushed the tickets/DM-38142 branch 2 times, most recently from c5e2dbf to ad884ba Compare February 28, 2023 03:32
Copy link
Contributor

@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.

  • Needs doc/changes file.
  • Is there documentation somewhere (panda.lsst.io maybe) where custom_lsst_setup needs to be mentioned?
  • I think this meets the user's request. However, in this or some other ticket, similar changes should be made to the bps_idf.yaml as this feature is not usdf specific (even though the value of custom_lsst_setup is different). Also, after pulling this out, can runnerCommand now only appear in the central bps_panda.yaml? (maybe another "extra_docker_env" bps setting needs to be introduced and only have a value at IDF?).

@@ -10,10 +10,13 @@ computeSite: SLAC
requestMemory: 2048
# PanDA does the scheduling based on memory request


custom_lsst_setup: ""
setupLSSTEnv: >
source /cvmfs/sw.lsst.eu/linux-x86_64/lsst_distrib/{LSST_VERSION}/loadLSST.bash;
pwd; ls -al;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why the "pwd; ls -al;" is in the middle of setupLSSTEnv. If the runnerCommand needs to be in a certain directory or have certain files there, those two commands would be better in the runnerCommand after setupLSSTEnv to give helpful debugging info in case the user's custom_lsst_setup commands change expected directory behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to runnerCommand

@zhaoyuyoung zhaoyuyoung merged commit 63bda7e into main Mar 1, 2023
@zhaoyuyoung zhaoyuyoung deleted the tickets/DM-38142 branch March 1, 2023 16:24
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

2 participants