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.

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

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

Closed
wants to merge 3 commits into from

Conversation

noirsoldats
Copy link
Contributor

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 noirsoldats marked this pull request as draft February 29, 2024 04:01
@noirsoldats
Copy link
Contributor Author

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.

@noirsoldats noirsoldats marked this pull request as ready for review February 29, 2024 09:03
@Commifreak
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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

let codeElems = $('code[data-container]:contains(' + element + ')');

That lead to false positives!

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

@Commifreak
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

I like the addition of the excluded/included attributes.

What do you mean?

@noirsoldats
Copy link
Contributor Author

Sorry, I meant excluded and internal attributes

@Commifreak
Copy link
Owner

Still dont got it 😅

so: do you think it includes all your ideas?

@c0d3m0nky
Copy link

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
Copy link
Owner

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

@c0d3m0nky
Copy link

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

Ah, gotcha. Thanks

@Commifreak
Copy link
Owner

Fixed by 4a704b1

@Commifreak Commifreak closed this Mar 12, 2024
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.

None yet

3 participants