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-30335: Minor fixes to execution butler #185

Merged
merged 4 commits into from May 27, 2021
Merged

DM-30335: Minor fixes to execution butler #185

merged 4 commits into from May 27, 2021

Conversation

timj
Copy link
Member

@timj timj commented May 25, 2021

No description provided.

timj added 4 commits May 26, 2021 08:54
…tler

Without this the predicted outputs that rely on new dimensions
(such as visit) can't be inserted.
It must be a directory so declare that it is one to avoid
confusion. Without this a trailing / is required which will
break if the execution butler directory does not yet exist.
# Need to ensure that the dimension records for outputs are
# transferred.
for _, dataIds in inserts.items():
exporter.saveDataIds(dataIds)
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be fine, but we should be on the lookout for it running into the problem @SimonKrughoff reported here where we export visit records but not visit definition records. I'm not sure exactly what he did differently.

# Requires that we use the dimension configuration from the original
# butler and not use the defaults.
config = Butler.makeRepo(root=outputLocation, config=config,
dimensionConfig=butler.registry.dimensions.dimensionConfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this is the difference between how I was doing things before with initializing the registry explicity vs switching to butler.makeRepo to make the registry in terms of dimensions being transfered.

Copy link
Member Author

Choose a reason for hiding this comment

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

We haven't changed dimensions config in a long time so I don't think this would have mattered. It was going to be a potential problem in the future though.

@timj timj merged commit 1bb3bd2 into master May 27, 2021
@timj timj deleted the tickets/DM-30335 branch May 27, 2021 20:56
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