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

Add Ombi Issues #74

Merged
merged 4 commits into from
Dec 30, 2018
Merged

Add Ombi Issues #74

merged 4 commits into from
Dec 30, 2018

Conversation

winbergoscar0
Copy link
Contributor

Fixes #70

Example Grafana table
dab6455eb75f38e5dee17d21c097bd5a

@dirtycajunrice
Copy link
Member

dirtycajunrice commented Dec 29, 2018

Hey, can you please strip this down to only an issue count? What the issues are aren’t relevant as if there are issues you need to go to Ombi to make any changes anyway. E.g. your first module but not your breakdown module

@winbergoscar0
Copy link
Contributor Author

Hi, of course I can do that, however.

I understand what you're saying and I personally won't use the detailed version, but others might find it useful. I'm just saying this since in the feature request @mastercpt said that a table with data would be good, and so might other users.

Would it not be better to turn the function off in the config if a user isn't interested in a more detailed overview. Maybe even disable it by default?

Just a thought 👍

@dirtycajunrice
Copy link
Member

@anderssonoscar0 there are many people who would like to use grafana for something that it’s not. And while we cannot stop them from privately implementing it without varken, we CAN refuse to inundate the influx varken db with information that does not fit the scope of our project or comply with our views of grafanas purpose.

Basically we have the “if you want to use it improperly that’s fine but we won’t help with that” mentality.

The code you have written is good, and should our position change on the subject it will be an easy splice to plug that function back in. We just don’t want to do so currently

@samwiseg0
Copy link
Member

samwiseg0 commented Dec 29, 2018

I agree with dirty. We are looking at this from a futures perspective. We want to make sure the project is sustainable, maintainable, and easy to use for the future. Also everything has to be QA whenever code is added/modified.

We want to make sure things we add are purposeful and well built. Having an option just because someone may want it is not in the spirit of what we want to accomplish. I am not saying this is not well built, because it is. But, it really does not serve enough of a purpose in my opinion.

@winbergoscar0
Copy link
Contributor Author

winbergoscar0 commented Dec 29, 2018

I understand! Removed 👍

Grafana example of whats left:
671685aae5178656c606de452da37f17

@dirtycajunrice
Copy link
Member

dirtycajunrice commented Dec 29, 2018

Awesome thanks. Ok only other change I see is that since it’s just integers a structure is not needed. If you reference the Ombi requests function, getting to the proper part of the response then typecasting an int in the influx payload

-Edit: I see now. I’ll merge tonight.

Sent with GitHawk

@dirtycajunrice dirtycajunrice merged commit f22c5eb into Boerderij:nightly Dec 30, 2018
@winbergoscar0 winbergoscar0 deleted the nightly branch December 31, 2018 01:18
@Boerderij Boerderij locked and limited conversation to collaborators Mar 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants