Skip to content

Add the --delete-local-file-after-upload to delete local file after upload#143

Merged
orgrim merged 1 commit intoorgrim:masterfrom
stephane-klein:sklein-remove-local-file-after-upload-to-object-storage
May 1, 2025
Merged

Add the --delete-local-file-after-upload to delete local file after upload#143
orgrim merged 1 commit intoorgrim:masterfrom
stephane-klein:sklein-remove-local-file-after-upload-to-object-storage

Conversation

@stephane-klein
Copy link
Contributor

Some users don't see any interest in consuming disk space on the server where pg_back is running when the archives have been saved remotely, for example on a secure Object Storage service.
This option allows to delete these intermediate files.

I chose the name --delete-local-file-after-upload for this parameter, but I am entirely open to modifying it if you have a more explicit suggestion, one that better conforms to the project's naming conventions, or that better describes its function.

From my perspective, it would make sense to enable this option by default when the upload functionality is activated. However, to avoid any breaking changes in behavior that might disrupt existing users, I preferred to set this option to false by default.

@stephane-klein
Copy link
Contributor Author

@orgrim What do you think?

stephane-klein added a commit to stephane-klein/pg_back-docker-sidecar that referenced this pull request Apr 14, 2025
This version is implemented with the following patch: orgrim/pg_back#143
stephane-klein added a commit to stephane-klein/pg_back-docker-sidecar that referenced this pull request Apr 14, 2025
This version is implemented with the following patch: orgrim/pg_back#143
stephane-klein added a commit to stephane-klein/pg_back-docker-sidecar that referenced this pull request Apr 14, 2025
This version is implemented with the following patch: orgrim/pg_back#143
Copy link
Owner

@orgrim orgrim left a comment

Choose a reason for hiding this comment

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

Hi,

my old sysadmin habits make me like shorter names for options, what about --delete-uploaded that takes "yes" or "no" (so that any state in the configuration file can be overridden on CLI?

Would you mind rebasing on the master branch, please? The CI has been fixed.

Regards

@l00ptr
Copy link
Collaborator

l00ptr commented Apr 23, 2025

is it possible to add some (basic) tests for that new flag ?

@stephane-klein stephane-klein force-pushed the sklein-remove-local-file-after-upload-to-object-storage branch from 8a244b0 to 450ae19 Compare April 25, 2025 09:08
Some users don't see any interest in consuming disk space on the server
where pg_back is running when the archives have been saved remotely,
for example on a secure Object Storage service.
This option allows to delete these intermediate files.
@stephane-klein stephane-klein force-pushed the sklein-remove-local-file-after-upload-to-object-storage branch from 450ae19 to 412eb25 Compare April 25, 2025 09:10
@stephane-klein
Copy link
Contributor Author

Would you mind rebasing on the master branch, please? The CI has been fixed.

@orgrim done

@stephane-klein
Copy link
Contributor Author

my old sysadmin habits make me like shorter names for options, what about --delete-uploaded that takes "yes" or "no" (so that any state in the configuration file can be overridden on CLI?

@orgrim done

Comment on lines +477 to +534
{
[]string{"--delete-uploaded", "yes"},
options{
Directory: "/var/backups/postgresql",
Format: 'c',
DirJobs: 1,
CompressLevel: -1,
Jobs: 1,
PauseTimeout: 3600,
PurgeInterval: -30 * 24 * time.Hour,
PurgeKeep: 0,
SumAlgo: "none",
CfgFile: "/etc/pg_back/pg_back.conf",
TimeFormat: timeFormat,
Encrypt: false,
CipherPassphrase: "",
WithRolePasswords: true,
Upload: "none",
Download: "none",
ListRemote: "none",
AzureEndpoint: "blob.core.windows.net",
B2ConcurrentConnections: 5,
DeleteUploaded: true,
},
false,
false,
"",
"",
},
{
[]string{"--delete-uploaded", "true"},
options{
Directory: "/var/backups/postgresql",
Format: 'c',
DirJobs: 1,
CompressLevel: -1,
Jobs: 1,
PauseTimeout: 3600,
PurgeInterval: -30 * 24 * time.Hour,
PurgeKeep: 0,
SumAlgo: "none",
CfgFile: "/etc/pg_back/pg_back.conf",
TimeFormat: timeFormat,
Encrypt: false,
CipherPassphrase: "",
WithRolePasswords: true,
Upload: "none",
Download: "none",
ListRemote: "none",
AzureEndpoint: "blob.core.windows.net",
B2ConcurrentConnections: 5,
DeleteUploaded: true,
},
false,
false,
"invalid value for --delete-uploaded: value must be \"yes\" or \"no\"",
"",
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@orgrim I've added these two tests.

I'm not sure of the quality of my implementation.

Does it suit you?

@stephane-klein
Copy link
Contributor Author

is it possible to add some (basic) tests for that new flag ?

See https://github.com/orgrim/pg_back/pull/143/files#r2059869288

@stephane-klein stephane-klein requested a review from orgrim April 25, 2025 09:14
@orgrim orgrim merged commit 1edbf9e into orgrim:master May 1, 2025
9 checks passed
@orgrim
Copy link
Owner

orgrim commented May 1, 2025

Merged. Thanks a lot for the contribution.

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.

3 participants