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

added functionality to remove successfully ploaded files; additionaly… #44

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

vlerkin
Copy link
Collaborator

@vlerkin vlerkin commented Dec 19, 2024

Cherry-picked changes (1 last commit from 668-joblogs branch):

Added logic to remove files:

After file was successfully uploaded to the S3-like-storage the file in the volume will be removed;
When code receives events from other existing jobs on the cluster it checks if files with its logs are present in the S3-like-storage, if not the code will attempt to collect logs, if the stdout is still available, if the file exists the code now checks if that file also still present in the volume and removes it since it is present in the S3-like-storage.
I think this functionality addresses your concerns.

… on a check when old jobs are present on the cluster and the code checks that the files of those jobs already exist in the S3 storage, added a file removal if it is still in the volume
@vlerkin vlerkin requested a review from wvengen December 19, 2024 11:05
@vlerkin
Copy link
Collaborator Author

vlerkin commented Dec 19, 2024

Hi Willem, instead of rebasing 20 commits I decided to create a new branch deriving from main and apply one commit that actually matters, since all other commits have already been merged into main.

Copy link
Member

@wvengen wvengen left a comment

Choose a reason for hiding this comment

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

Clear, thanks!

@vlerkin vlerkin merged commit 7a81737 into q-m:main Dec 19, 2024
5 checks passed
@wvengen
Copy link
Member

wvengen commented Dec 19, 2024

Thanks for merging!
Please prefix the PR number with "PR", and include an issue if possible in the commit line as well when merging, and keep the first line short when possible (additional details can be present on later lines). So somethine like this would have been appropriate:

Remove already uploaded files (#28, PR #44)

after they have been uploaded, and also when old jobs are present on the cluste the files of those jobs already exist on container storage

Otherwise 👍

@vlerkin
Copy link
Collaborator Author

vlerkin commented Dec 19, 2024

Sorry, got it

@wvengen wvengen mentioned this pull request Jan 21, 2025
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.

2 participants