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.

Sign up for GitHub

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

feat: transfer also removes empty log folders #463

Open
wants to merge 2 commits into
base: transition-to-runkit
Choose a base branch
from
Open

feat: transfer also removes empty log folders #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4462a2
feat: remove log folder if empty
eduardacoppo Feb 20, 2025
921f424
test: add tests for folder deletion
eduardacoppo Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion lib/syskit/cli/log_runtime_archive.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,24 @@ def self.transfer_dataset(
result = TransferDatasetResult.new(
complete: complete, transfer_results: transfer_results
)
log_transfer_results(dataset_path, result, logger: logger)
results = log_transfer_results(dataset_path, result, logger: logger)
remove_dataset_folder_if_empty(dataset_path, logger)
results
Comment on lines +188 to +190
Copy link
Member

Choose a reason for hiding this comment

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

I believe that there is a race condition here, especially for process servers: the folder will have only a info.yml until the remote system actually generates some other logs.

I actually believe that with the new system, the folder will be really empty in the end (no info.yml)

end

def self.remove_dataset_folder_if_empty(dataset_path, logger)
remaining_files = Dir.children(dataset_path) - ["info.yml"]
if remaining_files.empty?
begin
FileUtils.remove_dir(dataset_path)
logger.info("Deleted empty dataset folder: #{dataset_path}")
rescue StandardError => e
logger.error("Failed to delete empty dataset folder " \
"#{dataset_path}: #{e.message}")
end
else
logger.info("Skipping folder deletion: #{dataset_path} is not empty")
end
end

# Logs the transfer dataset results
Expand Down
65 changes: 65 additions & 0 deletions test/cli/test_log_runtime_archive.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,71 @@ def create_server(params)
assert result.success?, "transfer failed: #{result.message}"
end
end

describe "empty log folder deletion" do
it "transfers all files and deletes the folder if only info.yml " \
"remains" do
dataset = make_valid_folder("PATH")
make_random_file "test.0.log", root: dataset
make_random_file "test.1.log", root: dataset
make_random_file "info.yml", root: dataset

@process.process_dataset_transfer(
dataset, @params, @root, full: true
)

assert(File.exist?(@target_dir / "PATH" / "test.0.log"))
assert(File.exist?(@target_dir / "PATH" / "test.1.log"))
refute(File.exist?(dataset))
end

it "does not delete folder if other files remain" do
dataset = make_valid_folder("PATH")
make_random_file "test.0.log", root: dataset
make_random_file "test.1.log", root: dataset
make_random_file "info.yml", root: dataset
make_random_file "bla.txt", root: dataset

@process.process_dataset_transfer(
dataset, @params, @root, full: true
)

assert(File.exist?(@target_dir / "PATH" / "test.0.log"))
assert(File.exist?(@target_dir / "PATH" / "test.1.log"))
assert(File.exist?(dataset))
end

it "deletes the folder if it is completely empty" do
dataset = make_valid_folder("PATH")

@process.process_dataset_transfer(
dataset, @params, @root, full: true
)

refute(File.exist?(dataset))
end

it "does not delete folder if transfer fails" do
dataset = make_valid_folder("PATH")
make_random_file "test.0.log", root: dataset

result = RobyApp::LogTransferServer::LogUploadState::Result.new(
"/PATH", false, "message"
)

flexmock(LogRuntimeArchive)
.should_receive(:transfer_file)
.and_return(result)

results = LogRuntimeArchive.transfer_dataset(
dataset, @params, @root, full: true
)

refute results.success?
assert((dataset / "test.0.log").exist?)
assert(File.exist?(dataset))
end
end
end

describe "#ensure_free_space" do
Expand Down