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-44583: support manual config outputs in mocking system #420

Merged
merged 5 commits into from
May 30, 2024

Conversation

TallJimbo
Copy link
Member

@TallJimbo TallJimbo commented May 29, 2024

Checklist

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

TallJimbo added 2 commits May 29, 2024 19:37
Previously we just looked at the storage classes to identify config,
log, and metadata datasets that we shouldn't mock because they're
written by the execution system.  But this prevented tasks from being
able to have regular outputs using those storage classes; we *do* want
to mock those.
This works around the fact we can't necessarily see the full pipeline
graph in mock_pipeline_graph and hence can't identify when a special
unmocked output (e.g. a config) is read in by a later task in order to
mark the dataset type as unmocked in the later task.
Copy link

codecov bot commented May 29, 2024

Codecov Report

Attention: Patch coverage is 17.24138% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 80.92%. Comparing base (8d99519) to head (272ca26).

Files Patch % Lines
...ython/lsst/pipe/base/tests/mocks/_storage_class.py 22.22% 13 Missing and 1 partial ⚠️
...ython/lsst/pipe/base/tests/mocks/_pipeline_task.py 9.09% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #420      +/-   ##
==========================================
- Coverage   81.07%   80.92%   -0.16%     
==========================================
  Files          95       95              
  Lines       10833    10818      -15     
  Branches     2060     2064       +4     
==========================================
- Hits         8783     8754      -29     
- Misses       1705     1718      +13     
- Partials      345      346       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@erykoff erykoff left a comment

Choose a reason for hiding this comment

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

The code looks fine from what I understand, but I do not understand the mocking system of the pipeline graphs...

@erykoff
Copy link
Contributor

erykoff commented May 29, 2024

There might need to be some sort of uv patch required as well?

@timj
Copy link
Member

timj commented May 30, 2024

Fix for doc failure is on #419 with 083cfc4

Virtual env environment variable is no longer supported.
@TallJimbo TallJimbo merged commit 3934ca3 into main May 30, 2024
11 of 13 checks passed
@TallJimbo TallJimbo deleted the tickets/DM-44583 branch May 30, 2024 02:45
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.

3 participants