-
Notifications
You must be signed in to change notification settings - Fork 19
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
(OraklNode) Update restartFetcher #2241
Conversation
WalkthroughWalkthroughThe changes involve the removal of activation and deactivation functionalities for fetchers across multiple files, including the elimination of related HTTP routes and test cases. Additionally, modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant Controller
participant App
Client->>Router: POST /activate/:id
Router->>Controller: activate(c)
Controller->>App: startFetcherById(ctx, configId)
App-->>Controller: Fetcher started
Controller-->>Router: Response to client
Client->>Router: POST /deactivate/:id
Router->>Controller: deactivate(c)
Controller->>App: stopFetcherById(ctx, configId)
App-->>Controller: Fetcher stopped
Controller-->>Router: Response to client
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- node/pkg/admin/fetcher/controller.go (1 hunks)
- node/pkg/admin/fetcher/router.go (1 hunks)
- node/pkg/admin/tests/fetcher_test.go (2 hunks)
- node/pkg/fetcher/app.go (4 hunks)
- node/pkg/websocketfetcher/app.go (2 hunks)
Files skipped from review due to trivial changes (1)
- node/pkg/admin/fetcher/router.go
Additional context used
Learnings (1)
Common learnings
Learnt from: nick-bisonai PR: Bisonai/orakl#1280 File: node/pkg/fetcher/fetcher.go:267-267 Timestamp: 2024-03-19T13:22:13.702Z Learning: Data insertion in the Orakl project is handled by `./core`, not `./node`, and references within `./api` are considered irrelevant to changes made in the PR context. This highlights the architectural separation between `./api`, `./core`, and `./node` within the project.
Additional comments not posted (19)
node/pkg/admin/tests/fetcher_test.go (3)
Line range hint
8-21
: LGTM!The test function is correctly implemented and tests the start functionality as expected.
The code changes are approved.
Line range hint
23-36
: LGTM!The test function is correctly implemented and tests the stop functionality as expected.
The code changes are approved.
Line range hint
38-51
: LGTM!The test function is correctly implemented and tests the refresh functionality as expected.
The code changes are approved.
node/pkg/admin/fetcher/controller.go (3)
Line range hint
9-19
: LGTM!The function is correctly implemented and handles the start request as expected.
The code changes are approved.
Line range hint
21-31
: LGTM!The function is correctly implemented and handles the stop request as expected.
The code changes are approved.
Line range hint
33-43
: LGTM!The function is correctly implemented and handles the refresh request as expected.
The code changes are approved.
node/pkg/websocketfetcher/app.go (3)
109-109
: LGTM!The addition of the
cancel
field to theApp
struct is a good practice for managing context cancellation.The code changes are approved.
254-268
: LGTM!The modifications to the
Start
method improve the application's responsiveness to context cancellation.The code changes are approved.
273-277
: LGTM!The addition of the
Stop
method is a good practice for managing the application's lifecycle.The code changes are approved.
node/pkg/fetcher/app.go (10)
Line range hint
11-21
: LGTM!The function correctly initializes the
App
instance with the required components.The code changes are approved.
Line range hint
23-28
: LGTM!The function correctly initializes the app and starts all fetchers.
The code changes are approved.
Line range hint
30-51
: LGTM!The function correctly subscribes to the message bus and handles incoming messages.
The code changes are approved.
Line range hint
53-94
: LGTM!The function correctly handles incoming messages and performs actions based on the command. The removal of
ACTIVATE_FETCHER
andDEACTIVATE_FETCHER
cases aligns with the PR objectives.The code changes are approved.
Line range hint
96-111
: LGTM!The function correctly starts all fetchers and related components. The addition of
a.WebsocketFetcher.Start(ctx)
ensures that the websocket fetcher is started.The code changes are approved.
Line range hint
113-140
: LGTM!The function correctly stops all fetchers and related components. The addition of
a.WebsocketFetcher.Stop()
ensures that the websocket fetcher is stopped.The code changes are approved.
Line range hint
248-303
: LGTM!The function correctly initializes the app with configurations, fetchers, and related components.
The code changes are approved.
Line range hint
143-151
: LGTM!The function correctly starts a specific fetcher.
The code changes are approved.
Line range hint
213-225
: LGTM!The function correctly stops a specific fetcher.
The code changes are approved.
Line range hint
305-307
: LGTM!The function correctly returns a string representation of the app.
The code changes are approved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Description
Stop
func: include websocketFetcher Stop in fetcher app StopAll funcType of change
Please delete options that are not relevant.
Checklist before requesting a review
Deployment