MangoByte icon indicating copy to clipboard operation
MangoByte copied to clipboard

made loki connection optional

Open evansteelepdx opened this issue 2 years ago • 4 comments

Starting from scratch resulted in several errors due to missing values from the loki object in the default settings.json file

  • added base cases for Loki query methods
  • added check for base_url content in settings.json parsing to avoid errors
  • added Loki config object to default settings.json file creation

evansteelepdx avatar Apr 14 '22 20:04 evansteelepdx

resolves #64

evansteelepdx avatar Apr 14 '22 20:04 evansteelepdx

Thanks for the pull request! So most of the changes you made were around moving loki to be a thing included by default in config files but I think I'd prefer to go with the approach of not having the "loki" object added to the config file at all in cases where people arent using it.

Given that, I think that theres actually only one line that breaks the current mangobyte which is this one: LOKI_APPLICATION_NAME = settings.loki["application"]. We could fix this in a couple ways, either just adding an if to it so it only runs that line if settings.loki exists, or just have that constant be a part of the class when it gets initialized, or even just not create a variable for it and put it directly in the strings that use it.

mdiller avatar Apr 19 '22 04:04 mdiller

@mdiller I agree that there are probably too many checks in this case, and that we can eliminate all unhandled exceptions by checking on that line. However, since each bot action logs a request to the Loki endpoint, you'll still get a stacktrace from the Loki emitter that'll fill up your local logs quickly

requests.exceptions.MissingSchema: Invalid URL '/loki/api/v1/push': No scheme supplied. Perhaps you meant http:///loki/api/v1/push?

  • Example where base_url is blank

If you're fine with lots of local handled exceptions, then it's probably fine to keep in. I've added a new commit with the only 2 required change points:

  • settings.json file read
  • looping request in update_botstats

evansteelepdx avatar Apr 19 '22 23:04 evansteelepdx

Hey! Circling back to this issue and taking a fresh look at it, I think the only real problem here is the line in cogs/general.py that does LOKI_APPLICATION_NAME = settings.loki["application"]. That should only be getting called if loki exists, so if we just change that to:

if settings.loki:
  LOKI_APPLICATION_NAME = settings.loki["application"]

Then that should fix the problem. I think I'd like to keep the rest of the behavior the same, that way theres no problem if nobody adds a "loki" key in the settings.json, and then if they do add "loki" but they didnt do the rest of the args right, it should error at them, to let them know that theyre doing something wrong.

Sound good?

mdiller avatar Jun 21 '22 01:06 mdiller