Skip to content

restore: add fileregex parameter#1587

Merged
BareosBot merged 10 commits intobareos:masterfrom
SamuelBoerlin:restore-fileregex
Dec 7, 2023
Merged

restore: add fileregex parameter#1587
BareosBot merged 10 commits intobareos:masterfrom
SamuelBoerlin:restore-fileregex

Conversation

@SamuelBoerlin
Copy link
Contributor

@SamuelBoerlin SamuelBoerlin commented Nov 6, 2023 •
edited by sebsura
Loading

Thank you for contributing to the Bareos Project!

I'm currently using python-bareos to run automated restores and I'd like to filter by regex which files are restored. As far as I can tell this is currently not possible to do via restore command (except when records were pruned), hence this PR.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

Make sure you check/merge the PR using devtools/pr-tool to have some simple automated checks run and a proper changelog record added.

General
  • Is the PR title usable as CHANGELOG entry?
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
    Check backport line
  • Required backport PRs have been created
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault

@pstorz pstorz requested review from arogge and sebsura and removed request for arogge November 9, 2023 11:20
@sebsura sebsura self-assigned this Nov 10, 2023
sebsura
sebsura requested changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@sebsura sebsura left a comment

Choose a reason for hiding this comment

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

This is a great change. You need to fix your commit formatting though. pr-tool check reports the following problems:

4af68f0e0 systemtests:add RestoreDirectory variable to allow changing default restore directory: headline too long
b8c4d33d9 docs: add fileregex parameter to restore command documentation: headline too long

@SamuelBoerlin
Copy link
Contributor Author

SamuelBoerlin commented Nov 10, 2023

Is it okay now?
I tried running pr-tool check to confirm but it fails with Unknown JSON field: "headRefOid".

@sebsura
Copy link
Contributor

sebsura commented Nov 10, 2023 •
edited
Loading

Is it okay now? I tried running pr-tool check to confirm but it fails with Unknown JSON field: "headRefOid".

Did you run pipenv sync ? if not you can find instructions on how to setup pr-tool inside devtools/pip-tools/README.md. Are you working in a full bareos clone or did you do a shallow clone / single branch clone ?

@sebsura
Copy link
Contributor

sebsura commented Nov 10, 2023

pr-tool said the commits are fine now.

@SamuelBoerlin
Copy link
Contributor Author

SamuelBoerlin commented Nov 13, 2023

Is it okay now? I tried running pr-tool check to confirm but it fails with Unknown JSON field: "headRefOid".

Did you run pipenv sync ? if not you can find instructions on how to setup pr-tool inside devtools/pip-tools/README.md. Are you working in a full bareos clone or did you do a shallow clone / single branch clone ?

No, I did not run pipenv sync, maybe that's it. Thanks!

@SamuelBoerlin
Copy link
Contributor Author

SamuelBoerlin commented Nov 17, 2023

Just saw the Bareos 23 announcement and I'm looking forward to the release in the near future. Is there still any chance this could make it into Bareos 23?

@sebsura
Copy link
Contributor

sebsura commented Nov 23, 2023

Sorry for the late reply. Yes it is still possible to get it into 23. While we plan to release a preview of 23.0.0 very soon, it will still take some time for the real release.

While testing the new feature a bit I noticed the following problem:
The file tree is still getting build. This can take some time and selecting items in that tree can be very awkward since you may select items that do not match the supplied regex. We would like it to either behave similarly to file= or dir= in that it does not build the tree and requires no manual selection or instead the file tree needs to not insert items not matching the regex.

@SamuelBoerlin
Copy link
Contributor Author

SamuelBoerlin commented Nov 23, 2023

Sorry for the late reply. Yes it is still possible to get it into 23. While we plan to release a preview of 23.0.0 very soon, it will still take some time for the real release.

While testing the new feature a bit I noticed the following problem: The file tree is still getting build. This can take some time and selecting items in that tree can be very awkward since you may select items that do not match the supplied regex. We would like it to either behave similarly to file= or dir= in that it does not build the tree and requires no manual selection or instead the file tree needs to not insert items not matching the regex.

That's great, thanks.
I hadn't realized this behaviour and I totally agree with your suggestion. I think it'd make most sense to make it behave like file= or dir= where it doesn't ask the user to select files nor build the file tree. I'll try to implement that.

@SamuelBoerlin
Copy link
Contributor Author

SamuelBoerlin commented Nov 23, 2023

I've now changed fileregex to behave more like the file parameter. If no JobIds are specified then it selects the most recent JobIds (before specified date) to restore from without asking the user or building the file tree.

sebsura
sebsura requested changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@sebsura sebsura left a comment

Choose a reason for hiding this comment

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

The code changes are great. I added some comments to the doc & test. Once those are addressed, it will be approved.

@SamuelBoerlin
Copy link
Contributor Author

SamuelBoerlin commented Dec 4, 2023

Thanks for your suggestions. I've changed the file regex to something else so that it also contains files with special characters and whitespaces. There was also a small bug in check_restore_files_diff() where it would interpret the file names as regex pattern, causing it to fail e.g. for files with an unescaped backslash in it.

sebsura
sebsura reviewed Dec 6, 2023
View reviewed changes
@sebsura sebsura mentioned this pull request Dec 7, 2023
Merged
15 tasks
@BareosBot BareosBot merged commit d659c15 into bareos:master Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sebsura sebsura sebsura approved these changes

Assignees

@sebsura sebsura

Labels

requires backport to 23

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SamuelBoerlin @sebsura @BareosBot