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

Split to a Grafana Service #180

Merged
merged 44 commits into from
Apr 26, 2024
Merged

Split to a Grafana Service #180

merged 44 commits into from
Apr 26, 2024

Conversation

KeesCBakker
Copy link
Collaborator

@KeesCBakker KeesCBakker commented Apr 23, 2024

Partially fixes #177 in which we do a refactor to:

┌────────────────────────────┐   ┌────────────────────────┐    ┌────────────────────────┐ 
│ BOT                        │   │ ADAPTER                │    │ CUSTOM RESPONDER       │ 
│                            │   │                        │    │                        │ 
│ handles request from Hubot ├───► respond to a specific  ├────► influence the way the  │ 
│ responsible for response   │   │ chat platform          │    │ response is written    │ 
│ formatting                 │   │                        │    │                        │ 
└──────────────┬─────────────┘   └────────────────────────┘    └────────────────────────┘ 
               │                                                                          
┌──────────────▼─────────────┐                                                            
│ SERVICE                    │                                                            
│                            │                                                            
│ business rules for         │                                                            
│ querying of dashboards     │                                                            
└──────────────┬─────────────┘                                                            
               │                                                                          
┌──────────────▼─────────────┐                                                            
│  CLIENT                    │                                                            
│                            │                                                            
│  interaction with the      │                                                            
│  Grafana API over HTTP     │                                                            
└────────────────────────────┘

So in this PR we are doing the following:

  • Combining Adapter, GrafanaService and the GrafanaClient into a Bot class. This makes it a bit easier to interact with the package and not having to build your own class. I could not get the new Bot class to handle all the Hubot requests.
  • Improved failing test speeds. Tests should be finished withing 100ms. This was 2000ms, causing failing tests to run way too long for refactoring. Also: --forbid-only --forbid-pending when doing a commit or push, so we fail when we try to commit a mocka.only.
  • Improved test coverage by adding tests for pausing and unpausing all alerts.
  • The GrafanaService has all the business rules. One could use them without the bot (to collect multiple dashboards before sending them on).
  • Added some types to types.d.ts so JSDoc understands what we're doing.
  • With set setResponder it is easier for users to influence the way the dashboard is formatted (so we don't need to use middleware like https://keestalkstech.com/2023/08/improving-responsiveness-of-hubot-grafana/#putting-it-all-together anymore.

@KeesCBakker
Copy link
Collaborator Author

Interesting coverage. We might need to add some extra tests.

image

@KeesCBakker
Copy link
Collaborator Author

"It's not easy being green!" 😁

image

…akes a bit longer at my machine, so add some more timeout to prevent false negatives.
Copy link
Owner

@stephenyeargin stephenyeargin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't had a chance to load it up on to a running Hubot, but the code looks great.

.husky/pre-commit Show resolved Hide resolved
package.json Show resolved Hide resolved
src/adapters/Adapter.js Show resolved Hide resolved
@KeesCBakker
Copy link
Collaborator Author

Will test it on a live Hubot today. Have a great new feature I want to add to our bot, so at the end of the day we'll know.

@KeesCBakker
Copy link
Collaborator Author

KeesCBakker commented Apr 25, 2024

Did some testing and some further refactoring. All my tests with a live Hubot were also OK.

image

@stephenyeargin, we can merge if you don't see anything crazy.

@stephenyeargin
Copy link
Owner

Looks good to me, I'll let you do the honors. I see you're sneaking it a bit of TypeScript. 😉

@KeesCBakker
Copy link
Collaborator Author

Looks good to me, I'll let you do the honors. I see you're sneaking it a bit of TypeScript. 😉

Hahaha. Yeah the d.ts files work great together with JS Docs.

@KeesCBakker KeesCBakker merged commit c932fe4 into main Apr 26, 2024
4 checks passed
@KeesCBakker KeesCBakker deleted the feature/grafana-service branch April 26, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Let's do some big refactoring
2 participants