pghoard icon indicating copy to clipboard operation
pghoard copied to clipboard

Rommel layco receivelog respects sighup

Open RommelLayco opened this issue 2 years ago • 2 comments

About this change - What it does

This modifies the receivexlog to use the updated configuration settings following a SIGHUP

This is a continuation of the work done in https://github.com/aiven/pghoard/pull/530

Why this way

When SIGHUP is issued, self.config is refreshed in all threads, but without this change, the new values are disregarded. This change ensures the updated config object is always used.

RommelLayco avatar Oct 17 '22 03:10 RommelLayco

The test that is currently failing are

test/test_storage.py::test_storage_local_with_prefix test/test_storage.py::test_storage_local_no_prefix

It looks like this rohum commit broke the test https://github.com/aiven/rohmu/commit/b4861247ea3e5ca364566416d3415f1a535f97dd as it fails to save the metadata now.

RommelLayco avatar Oct 17 '22 05:10 RommelLayco

The test that is currently failing are

test/test_storage.py::test_storage_local_with_prefix test/test_storage.py::test_storage_local_no_prefix

It looks like this rohum commit broke the test aiven/rohmu@b486124 as it fails to save the metadata now.

I am not sure how to fix this? The store_file_from_disk is called twice https://github.com/aiven/pghoard/blob/f85068c09a6e7db5842bae0aef87cab592b28e38/test/test_storage.py#L48-L49

This method calls save_metadata in rohmu, it looks like the problem is that due to the metadata file exists from the first call, and the atomic_opener can't seem to handle a file already existing? Is that the correct behaviour, don't we want to be able to overwrite the contents of an existing metadata file?

If not is the key provided in the snippet wrong? should it the second key have a different name like in the lines that follow

https://github.com/aiven/pghoard/blob/f85068c09a6e7db5842bae0aef87cab592b28e38/test/test_storage.py#L51-L52

The directories are NONEXISTENT-b/x1 and NONEXISTENT-a/x1 (b and a)

RommelLayco avatar Oct 17 '22 05:10 RommelLayco

Looks like the test were failing due to rohum. This has been fixed here https://github.com/aiven/rohmu/pull/76

RommelLayco avatar Oct 18 '22 01:10 RommelLayco