search

thelastWallE / OctoprintKlipperPlugin

A plugin for a better integration of Klipper into OctoPrint.
GNU Affero General Public License v3.0
65 stars 14 forks source link

Implement Configpath and Logpath Validation #8

Closed AliceGrey closed 3 years ago

AliceGrey commented 4 years ago

Added JS popup handler. Added basic configpath and logpath validation when settings are saved. Resolves issue #1 and #3

ollyfg commented 4 years ago

Hi there! Thanks for all your hard work taking over this awesome plugin!

Just reading the diff, and it looks like this PR will force the logpath to be an existing file in the filesystem - is that the behaviour you wanted?

It probably makes sense to at least warn people that it doesn't exist, but is it worth making that warning bypassable in the case of the logpath? It is useful to be able to set the logpath to /foo/bar/2020-10-01.log, without having to ssh in and touch /foo/bar/2020-10.log first.

Sorry I can't edit this PR, but I'm happy to file another PR once this is merged.

ssorgatem commented 4 years ago

Maybe a better alternative to os.path.isfile(filepath) would be something like: try: with open(filepath, 'a') as foo: pass except Exception as e: self.sendMessage("errorPopUp","warning", "OctoKlipper Settings", "Klipper {} cannot be written to! ({})".format(filepath, e))

This has the advantadge of checking whether the file can be created and written to

thelastWallE commented 3 years ago

Merged into 0.3.4rc1 The warning popup when there is no log file is shown on the dialog for analyzing the klipper log files. For now the path gets saved even if there isn't a log file.