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.

Sign up for GitHub

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

Jump to bottom

DM-33889: Pass cloud, site, queue (patterns) to PanDA appropriately #10

Merged
merged 31 commits into from Jun 29, 2022
Merged

DM-33889: Pass cloud, site, queue (patterns) to PanDA appropriately #10

merged 31 commits into from Jun 29, 2022

Conversation

wguanicedew
Copy link
Contributor

@wguanicedew wguanicedew commented May 29, 2022 •
edited by timj

Requires idds 0.11.0

Checklist

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

timj
timj approved these changes May 31, 2022
View reviewed changes
Copy link
Member

@timj timj left a comment

Choose a reason for hiding this comment

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

I'll leave @MichelleGower to comment specifically from the BPS point of view.

I see some formatting commits that should be squashed before merging. Consider using pre-commit to ensure that black runs before you can commit.

config/bps_idf.yaml
@@ -11,15 +11,17 @@ dockerImageLocation: "us-central1-docker.pkg.dev/panda-dev-1a74/"
maxJobsPerTask: 70000

#IDF PanDA specific settings:
computeSite: LSST
computeCloud: "LSST"
Copy link
Member

Choose a reason for hiding this comment

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

We should add the yaml linting github action to this package.

https://github.com/lsst/.github/blob/main/workflow-templates/yamllint.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how to do this one.

config/bps_idf.yaml Outdated Show resolved Hide resolved
config/bps_idf.yaml Outdated Show resolved Hide resolved
config/bps_idf.yaml Outdated Show resolved Hide resolved
doc/changes/DM-33889.misc.rst Outdated Show resolved Hide resolved
python/lsst/ctrl/bps/panda/panda_service.py Show resolved Hide resolved
@timj timj changed the title tickets/DM-33889: add cloud, priority, prodsourcelabel support (require idds 0.11.0) DM-33889: add cloud, priority, prodsourcelabel support May 31, 2022
MichelleGower
MichelleGower approved these changes May 31, 2022
View reviewed changes
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.

There are a couple other values that can be fixed in this ticket (e.g., walltime and attempts) and a few minor cleanup bits. Merge approved after addressing @timj and my comments.

config/bps_idf.yaml Outdated Show resolved Hide resolved
config/bps_idf.yaml Outdated Show resolved Hide resolved
python/lsst/ctrl/bps/panda/idds_tasks.py Outdated Show resolved Hide resolved
python/lsst/ctrl/bps/panda/idds_tasks.py Show resolved Hide resolved
python/lsst/ctrl/bps/panda/idds_tasks.py Outdated Show resolved Hide resolved
config/bps_idf.yaml Outdated Show resolved Hide resolved
@mxk62 mxk62 changed the title DM-33889: add cloud, priority, prodsourcelabel support DM-33889: Pass cloud, site, queue (patterns) to PanDA appropriately Jun 22, 2022
@mxk62 mxk62 force-pushed the tickets/DM-33889 branch 2 times, most recently from 6541851 to 8f5e81d Compare June 22, 2022 20:30
@mxk62 mxk62 force-pushed the tickets/DM-33889 branch 3 times, most recently from b8db645 to 4e260f4 Compare June 22, 2022 23:30
@timj timj mentioned this pull request Jun 23, 2022
Merged
2 tasks
@codecov
Copy link

codecov bot commented Jun 24, 2022 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (main@ac57d26). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #10   +/-   ##
=======================================
  Coverage        ?   35.28%           
=======================================
  Files           ?        9           
  Lines           ?      479           
  Branches        ?       79           
=======================================
  Hits            ?      169           
  Misses          ?      307           
  Partials        ?        3

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac57d26...0b4acd7. Read the comment docs.

mxk62
mxk62 reviewed Jun 24, 2022
View reviewed changes
setup.cfg Outdated Show resolved Hide resolved
@mxk62 mxk62 merged commit 0005c0f into main Jun 29, 2022
@mxk62 mxk62 deleted the tickets/DM-33889 branch June 29, 2022 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timj timj timj approved these changes

@mxk62 mxk62 mxk62 left review comments

@MichelleGower MichelleGower MichelleGower approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@wguanicedew @timj @mxk62 @MichelleGower