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-31251: Allow butler.ingest to work with execution butler #551

Merged
merged 5 commits into from Aug 2, 2021

Conversation

timj
Copy link
Member

@timj timj commented Jul 30, 2021

Checklist

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

tgtLocation = None
else:
raise RuntimeError(f"Unexpected transfer mode encountered: {transfer} for"
f" URI {srcUri}")
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems like this whole if block is combining two different bits of logic together. It might be the right thing, but I am having a bit of trouble following it, not knowing what some of the results of calls will be.

You have changed to pathInStore is None and transfer is None is this implying that you are using something like True, False, and None as conditions in normal operation? Is it ok that if pathInStore is None, transfer is not None, for the if pathInStore to evaluate to false based on None being false-y?

I think I understand what you are doing and I think it is fine, but I am sufficiently uncertain, that I want to highlight it for a second look by you.

Copy link
Member Author

Choose a reason for hiding this comment

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

@TallJimbo used transfer=None to mean "the files are already in the right place" and no transfer is needed. For split mode I need to handle the case where some are in and some are out. For the files inside the target location is relative to the datastore root. For the files outside there is also no transfer but that is "direct" mode and is handled by setting the target location to None (so the code below knows to use the source URI directly). It is combining two bits of logic but the None mode is most of the logic and it didn't seem like it was reasonable to add a new split path as a separate check that would duplicate the None -- the only change is to set that target URI to None in split mode.

No longer used since tags were removed in DM-27153 and
commit 040943b
Add the check for pre-existing datasets in registry but no
file in datastore.
This mode allows an ingest to have some files that are inside
the datastore already and some that are outside. This is a common
scenario for execution butler where raws are in a shared location
outside of a datastore but calibrations and other products are
inside the butler datastore.

"auto" mode will choose the "split" option in that case.

This mode is effectively transfer=None for files inside the datastore
and transfer=direct for those outside the datastore.
@timj timj merged commit 023bc29 into master Aug 2, 2021
@timj timj deleted the tickets/DM-31251 branch August 2, 2021 17:28
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