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

remove the provenance file from the dataset #4

Merged
merged 3 commits into from
Jun 3, 2024

Conversation

KochTobi
Copy link
Member

@KochTobi KochTobi commented Jun 3, 2024

removes the provenance files, moves the folder into the dataset and writes the file again if something went wrong.

See #3 for details on the reason for this change.

removes the provenance files, moves the folder into the dataset and writes the file again if something went wrong.
@KochTobi KochTobi requested a review from a team as a code owner June 3, 2024 11:33
@KochTobi KochTobi changed the base branch from main to development June 3, 2024 11:34
@github-actions github-actions bot added the fix label Jun 3, 2024
sven1103
sven1103 previously approved these changes Jun 3, 2024
try {
transactionV2.moveFile(transactionV2.getIncoming().getAbsolutePath(), dataSet);
} catch (Exception e) {
Files.write(provenanceFile.toPath().toAbsolutePath(), buffer);
Copy link
Member

Choose a reason for hiding this comment

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

I'm like 99% sure openBIS will automatically recover deleted files (I think it stores a copy of the original folder) on failure, so I advise against writing anything after an exception the ETL service - I'm not sure how this will influence the behaviour

Copy link
Member

@wow-such-code wow-such-code left a comment

Choose a reason for hiding this comment

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

I'm not sure about the catch case, see my comment. Other than that the code looks good

@KochTobi
Copy link
Member Author

KochTobi commented Jun 3, 2024

@wow-such-code so the recovery is not needed?

Co-authored-by: wow-such-code <[email protected]>
@wow-such-code
Copy link
Member

@wow-such-code so the recovery is not needed?

Yes, it will recover by itself in an error case. The only case where files can vanish is if the script runs through without any errors and without moving files.

@KochTobi KochTobi merged commit 0fb3f04 into development Jun 3, 2024
3 checks passed
@KochTobi KochTobi deleted the fix/ignore-provenance-file branch June 3, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants