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

Libreant starts also if it fails to read the conf file #255

Closed
leophys opened this issue Mar 3, 2016 · 6 comments · Fixed by #260
Closed

Libreant starts also if it fails to read the conf file #255

leophys opened this issue Mar 3, 2016 · 6 comments · Fixed by #260
Labels

Comments

@leophys
Copy link
Member

leophys commented Mar 3, 2016

As mentioned in the title, libreant boots up also if it explicitly fails to read the selected configuration file

% libreant -s libreant-conf.json
2016-03-03 10:42:59,360 [presets.presetManager] [ERROR] Failed to load preset: "/home/leo/libreant-conf.json" [ Bad format: missing field 'id' ]
Listening on http://0.0.0.0:5000/
@boyska
Copy link
Member

boyska commented Mar 3, 2016

That's not exactly true. If the settings file is missing or not a valid JSON, libreant will correctly abort.
In your example the problem is about a preset that is not working. Could you help me reproduce the bug giving the content of the preset?

@boyska boyska added the bug label Mar 3, 2016
@leophys
Copy link
Member Author

leophys commented Mar 3, 2016

You're right. The fact is that I misinterpreted the meaning of the field PRESET_PATHS in the configuration json file as the path of the file itself. That doesn't make any sense.
I will close this.

@leophys leophys closed this as completed Mar 3, 2016
@ael-code
Copy link
Member

ael-code commented Mar 4, 2016

On 03/03/2016 03:29 PM, BoySka wrote:

That's not exactly true. If the settings file is missing or not a valid
JSON, |libreant| will correctly abort.
In your example the problem is about a preset that is not working. Could
you help me reproduce the bug giving the content of the preset?

Did you tested what you're saying?

we test only if file exists through click facilities, but this is not
enough.

I've tested the following two cases:

  • Not existing settings passed as environment variable
    LIBREANT_SETTINGS=/not/existing libreant -d.
  • Not enough permission on config file, both as env variable and cli
    argument.
  • File with wrong json syntax, both as env variable and cli.

In all the previous cases libreant is logging a warning message but then
it starts anyway.

@ael-code ael-code reopened this Mar 4, 2016
@boyska
Copy link
Member

boyska commented Mar 4, 2016

ael:

BoySka wrote:

That's not exactly true. If the settings file is missing or not a valid
JSON, |libreant| will correctly abort.
In your example the problem is about a preset that is not working. Could
you help me reproduce the bug giving the content of the preset?

Did you tested what you're saying?

I tested with a nonexisting file, and the webserver won't start.
If the file is not valid JSON, an exception will be given, but you're
right, the program will start anyway.

I've tested the following two cases:

  • Not existing settings passed as environment variable
    LIBREANT_SETTINGS=/not/existing libreant -d.
  • Not enough permission on config file, both as env variable and cli
    argument.
  • File with wrong json syntax, both as env variable and cli.

In all the previous cases libreant is logging a warning message but then
it starts anyway.

You're right, and I don't know if this is correct or wrong. I am sure
that if it is passed as a cli argument it must fail (and now it only
partially does).
I am not sure about the correct behaviour for nonexisting
$LIBREANT_SETTINGS
I guess that for existing but non-json $LIBREANT_SETTINGS exiting
would be the best thing to do.

@ael-code
Copy link
Member

ael-code commented Mar 5, 2016

On 03/04/2016 03:23 PM, BoySka wrote:

ael:

I've tested the following two cases:

  • Not existing settings passed as environment variable
    LIBREANT_SETTINGS=/not/existing libreant -d.
  • Not enough permission on config file, both as env variable and cli
    argument.
  • File with wrong json syntax, both as env variable and cli.

In all the previous cases libreant is logging a warning message but then
it starts anyway.

You're right, and I don't know if this is correct or wrong. I am sure
that if it is passed as a cli argument it must fail (and now it only
partially does).
I am not sure about the correct behaviour for nonexisting
$LIBREANT_SETTINGS
I guess that for existing but non-json $LIBREANT_SETTINGS exiting
would be the best thing to do.

I would exit also for the non-existing scenario.
Can you explain why you wouldn't?

@boyska
Copy link
Member

boyska commented Mar 5, 2016

ael:

On 03/04/2016 03:23 PM, BoySka wrote:

ael:

I've tested the following two cases:

  • Not existing settings passed as environment variable
    LIBREANT_SETTINGS=/not/existing libreant -d.
  • Not enough permission on config file, both as env variable and cli
    argument.
  • File with wrong json syntax, both as env variable and cli.

In all the previous cases libreant is logging a warning message but then
it starts anyway.

You're right, and I don't know if this is correct or wrong. I am sure
that if it is passed as a cli argument it must fail (and now it only
partially does).
I am not sure about the correct behaviour for nonexisting
$LIBREANT_SETTINGS
I guess that for existing but non-json $LIBREANT_SETTINGS exiting
would be the best thing to do.

I would exit also for the non-existing scenario.
Can you explain why you wouldn't?

No, I cannot. I was unsure and had a feeling that it would be wrong, but
you're probably right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants