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-31255: Fix log-to-butler failures with --skip-existing #132

Merged
merged 2 commits into from Jul 30, 2021

Conversation

andy-slac
Copy link
Collaborator

Add special logic to detect cases when we don't want to save log records
to butler. Removes temporary file in that case or when ingest does not
work. When ingest is not supported we now store an empty records object
to make sure that dataset exists for consistency with other cases.

Checklist

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

Add special logic to detect cases when we don't want to save log records
to butler. Removes temporary file in that case or when ingest does not
work. When ingest is not supported we now store an empty records object
to make sure that dataset exists for consistency with other cases.
" datastore can not ingest files, empty record list is stored instead.")
records = ButlerLogRecords.from_records([])
butler.put(records, ref[0])
# And remove the file, but ignore errors
Copy link
Member

Choose a reason for hiding this comment

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

Could maybe put this removal in a finally clause.

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.

Looks good to me. Thanks for fixing up my problem.

@@ -450,6 +465,15 @@ def writeLogRecords(self, quantum, taskDef, butler):
# Remove the handler so we stop accumulating log messages.
logging.getLogger().removeHandler(self.log_handler)

if not store:
# do not store anything but remove file
if filename is not None:
Copy link
Member

@timj timj Jul 29, 2021

Choose a reason for hiding this comment

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

what about:

if store and other tests:
    # store it

if filename:
    # remove it

would work wouldn't it? Then there would not be the os.remove code duplication logic.

@andy-slac andy-slac merged commit 41d63b6 into master Jul 30, 2021
@andy-slac andy-slac deleted the tickets/DM-31255 branch July 30, 2021 02:25
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