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

Publish BACKUPMON v1.8.17 #50

Merged
merged 19 commits into from
Jul 19, 2024
Merged

Publish BACKUPMON v1.8.17 #50

merged 19 commits into from
Jul 19, 2024

Conversation

ViktorJp
Copy link
Owner

No description provided.

Martinski4GitHub and others added 19 commits July 10, 2024 01:51
New code that enables checking multiple website URLs to download the Custom Email Library Script, just in case the first URL is not available for any reason.
Code improvements.
Fine-tuning.
Code improvements & fine-tuning.
More code improvements & fine-tuning.
Fine-tuning.
Check multiple URLs to download Custom Email Library
Added new "-veryquiet" parameter that can be used when checking for & downloading updates for the custom email library script.
Added "-veryquiet" parameter to email library.
@ViktorJp ViktorJp merged commit bda2635 into main Jul 19, 2024
cemailQuietArg="-verbose"
cemailCheckArg="-versionCheck"
fi
_CheckForCustomEmailLibraryScript_ "$cemailCheckArg" "$cemailQuietArg"
Copy link
Contributor

Choose a reason for hiding this comment

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

WRT lines 1160 to 1167:
Is there any specific reason for having the setup & call to the "_CheckForCustomEmailLibraryScript_ " function before sending a test email? IOW, what was the goal for placing the additional call here?

It should not be necessary at all, and it's rather redundant since it was already done at the start of script execution.

Copy link
Owner Author

Choose a reason for hiding this comment

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

It was actually my original intention for this section to be the piece that originally downloads the library, so that we don't take up space on people's routers that don't have amtm email enabled. But I need to refine this piece a bit more.

@Martinski4GitHub
Copy link
Contributor

Just to clarify, the primary goal of placing the call to check for updates to the custom email library at the start of the script is to minimize the number of calls. In previous versions of backupmon, you had placed the code to the library in multiple places which was unnecessary and inefficient. My latest changes are a way to rectify that.

@ViktorJp
Copy link
Owner Author

For the piece at the beginning, I actually forgot to qualify running this with an "if [ $AMTMEMAIL -eq 1 ]", in order to kick this off, again to weed out needing to download if this feature is turned off. But once it's yes, this would be the only place it gets updated. The piece under the config is primarily for initial install, using the verbose mode to give visual feedback.

@Martinski4GitHub
Copy link
Contributor

For the piece at the beginning, I actually forgot to qualify running this with an "if [ $AMTMEMAIL -eq 1 ]", in order to kick this off, again to weed out needing to download if this feature is turned off. But once it's yes, this would be the only place it gets updated. The piece under the config is primarily for initial install, using the verbose mode to give visual feedback.

Ah, OK, I understand now. The "if [ $AMTMEMAIL -eq 1 ]" conditional was the missing piece for the call at the start of script execution. The other call to download the custom email library right after the user has selected to enable email notifications but before the test email prompt is the right place for it.

Your latest update version "1.8.18" includes the above changes so it's all set up nicely now. Excellent!!

@Martinski4GitHub
Copy link
Contributor

@ViktorJp,

BTW, there are still some references to "Backup Frequency" that should be changed to "Backup Retention" for consistency throughout the UI menus.

Just FYI.

Backup Frequency

@ViktorJp
Copy link
Owner Author

Dangit! Thanks for the catch. 😋

@ViktorJp
Copy link
Owner Author

ViktorJp commented Jul 20, 2024

@Martinski4GitHub ... it seems "if [ -z "${amtmIsEMailConfigFileEnabled:+xSETx}" ]" is coming back as null, causing a condition that the Email Library Script is not being found. Any ideas? I put some screenshots/examples on SNB as well...

eTgx00l

@Martinski4GitHub
Copy link
Contributor

@Martinski4GitHub ... it seems "if [ -z "${amtmIsEMailConfigFileEnabled:+xSETx}" ]" is coming back as null, causing a condition that the Email Library Script is not being found. Any ideas? I put some screenshots/examples on SNB as well...

<
The particular "test email" section shown in the screenshot does not have the "if [ $AMTMEMAIL -eq 1 ]" conditional check so essentially you're asking the user to test an option that has not yet been enabled, and since it's not enabled, the custom email library has not been LOADED into memory, thus it's not FOUND.

The fix is to predicate the "test email" option on "email notifications" being previously enabled by the user; otherwise, the test cannot be performed.

@ViktorJp
Copy link
Owner Author

ViktorJp commented Jul 20, 2024

But for me it is 1, or enabled, so it should have loaded the library?

By running the check update at the beginning of the script, it should load the library right?

@Martinski4GitHub
Copy link
Contributor

But for me it is 1, or enabled, so it should have loaded the library?

By running the check update at the beginning of the script, it should load the library right?

If the variable AMTMEMAIL is set to 1 by the time the code to check for the email library is executed, then yes it will be loaded into memory. The problem with the "email test" section is that the variable is not even checked, so email notifications may or may not have been enabled yet.

@Martinski4GitHub
Copy link
Contributor

@ViktorJp,

I just ran a simple test by typing "/jffs/scripts/backupmon.sh -setup" and the variable AMTMEMAIL is still set to 0 when the code to check for the email library at the start of script execution is reached, even though I have "email notifications" enabled on a previous run.

This indicates that the variable AMTMEMAIL is likely not yet set correctly based on the current configuration settings.

@ViktorJp
Copy link
Owner Author

I think that was it... Your code is probably executing before I'm calling my config file variables. Thank you Martinski!

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.

2 participants