Commifreak / unraid-appdata.backup

UNRAID AppData backup plugin
21 stars 1 forks source link

Initial fix for 'Multi mapping detected' issue. #23

Closed noirsoldats closed 6 months ago

noirsoldats commented 7 months ago

I want to continue working on this and and make it where volume paths that are excluded from backup get ignored too but that'll take a bit more work. This fixes the majority of the problems with things incorrectly being listed as 'multi mapping detected'.

noirsoldats commented 7 months ago

Ok, this needs some more testing, but the functionality should now mark things as 'multi mapping detected' only if they are being backed up. If they are excluded they are not counted, and if they are external they aren't counted unless the container is configured to backup external.

Commifreak commented 7 months ago

Thanks for your input. Ill check this asap. Some few changes I would implement.

TBH I did not saw the initial issue with my detection and had no time for further test case setup. I firstly wanted to start understanding why my initial code did not worked as intended.

noirsoldats commented 7 months ago

The cause of the issue was the path matching being partial and so if a container had a mapping for /mnt/user (EG: QDirStat and many others) then EVERYTHING would match basically.

Commifreak commented 7 months ago

I searched multiple times but you are right! The issue is not on the determination part but on the warning-display part!

https://github.com/Commifreak/unraid-appdata.backup/blob/9e2655fb0ea66937e6615085a3b090b557892fd6/src/pages/content/settings.php#L1027

That lead to false positives!

I check your change soon but I add some changes to it.

Commifreak commented 7 months ago

I did some tests and changed the basic code a bit to have some flags like excluded and internal attributes in place. This reduces the need for some JS checks.

My recent changes include all of your ideas, though. Could you test it as well? There will be a new forum post in the next minutes. a one liner for people to test it.

noirsoldats commented 7 months ago

I've been using the patched version for several days now and it seems to be working very well.. I like the addition of the excluded/included attributes.

Commifreak commented 7 months ago

I like the addition of the excluded/included attributes.

What do you mean?

noirsoldats commented 7 months ago

Sorry, I meant excluded and internal attributes

Commifreak commented 7 months ago

Still dont got it 😅

so: do you think it includes all your ideas?

c0d3m0nky commented 6 months ago

Hey, applied the patch using the script in this forum post and found what seems to be a bug

https://forums.unraid.net/topic/137710-plugin-appdatabackup/?do=findComment&comment=1382170

I have Plex and Tautulli, I thought "let me remove the exclusion from Tautulli and see if this patch flags the shared path"

It did not

Plex: image

Tautulli: image

Commifreak commented 6 months ago

It is not designed to detect those matches. It detect exact matches only.

c0d3m0nky commented 6 months ago

It is not designed to detect those matches. It detect exact matches only.

Ah, gotcha. Thanks

Commifreak commented 6 months ago

Fixed by 4a704b12cadf936be6e719620e7672e14a0f04e9