jssimporter / python-jss

python-jss is deprecated. Please see the wiki for alternatives.
GNU General Public License v3.0
102 stars 41 forks source link

Using own preferences file doesn't work if the file doesn't exist #71

Open geoffklee opened 6 years ago

geoffklee commented 6 years ago

Perhaps this is the intended behaviour, but if I call JSSPrefs() with my own (non-existent) plist file as preferences_file argument, I'd expect it to act the same way as it does in the default case, and ask me for the information needed to create the file.

Instead, it throws an exception if the file isn't found. Is this intended? It's these lines:

https://github.com/sheagcraig/python-jss/blob/e1982466bd7e971f27525cbc31024590a456df66/jss/jss_prefs.py#L110-L112

I also suspect that the call to self.init() at the bottom of init() should include self.preferences_file as the preferences_file argument:

https://github.com/sheagcraig/python-jss/blob/e1982466bd7e971f27525cbc31024590a456df66/jss/jss_prefs.py#L132

Otherwise it tries to read the default plist. Am I missing something?

sheagcraig commented 6 years ago

Hi @gkluoe I'm not actually sure I ever thought of that situation. The code you reference in the first example was intended (I think) to handle a first time user starting up a JSSPrefs object. As it stands, this was written to try to reasonably detect that situation.

Perhaps a more appropriate test would be simply: if os.path.exists(os.path.expanduser(preferences_file or '')):

My suspicion is that partially, the reason for the first if preferences file is not None is that the following test would throw an exception if you give None to os.path,expanduser.

If you have a proposal or PR for how this should work, throw it at me.

As for the second code snippet, I don't remember why this is set up like this; but I suspect that you're right and it should be called with the preferences_file argument. I would just make sure it can't get into an infinite loop like this.