search

stephenyeargin / hubot-grafana

📈🤖 Query Grafana dashboards
http://docs.grafana.org/tutorials/hubot_howto/
MIT License
154 stars 48 forks source link

Refactor platform specific actions to adapters? #165

Closed KeesCBakker closed 12 months ago

KeesCBakker commented 1 year ago

Is your feature request related to a problem? Please describe. Not really a problem, but it might make maintenance a easier.

We make some choices based on the adapter:

So what if we would refactor this into some sort of an adapter? The adapter could:

Our script just initializes the right adapter (based on the robot.adapterName) and uses its methods. The implementations are neatly separated into their own classes.

Describe the solution you'd like Some separation of concerns.

Describe alternatives you've considered N/A

Additional context Would love to work on something like that.

KeesCBakker commented 1 year ago

@stephenyeargin, if you have some time, I might have something you can take a look at: https://github.com/KeesCBakker/hubot-grafana/pull/2

stephenyeargin commented 1 year ago

Looks like it takes care of the tagged issues, thank you! You should have write access now to this repository if you'd rather create branches there (so that the test suite runs against PRs).

KeesCBakker commented 1 year ago

Yeah, upon testing I found some problems. Looks like the S3 route works, but the other routes can never work.

The grafanaDashboardRequest() on the Slack upload (https://github.com/stephenyeargin/hubot-grafana/blob/main/src/grafana.js#L611) can never work, as it will not return a URL but is a callback, as is defined here: https://github.com/stephenyeargin/hubot-grafana/blob/main/src/grafana.js#L726-L735

Trying to fix that as well, but Slack won't led me upload files.

Other broken cases:

The tests currently are not catching that those uploadTo things are broken. Do you manually test them?