Add gocron scheduler for devbuild polling

[wuhuizuo]

• Integrated gocron scheduler to handle periodic polling of devbuild status.
• Updated NewRootForMessage to accept and initialize the scheduler.
• Modified runCommandDevbuildTrigger to schedule polling tasks.
• Refactored devbuild polling logic into separate functions.

[ti-chi-bot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from wuhuizuo, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[gemini-code-assist]

Hello @wuhuizuo, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request introduces a gocron scheduler to periodically poll the status of devbuilds. It modifies the NewRootForMessage function to accept and initialize the scheduler, and updates runCommandDevbuildTrigger to schedule polling tasks. The devbuild polling logic has also been refactored into separate functions for better organization and reusability.

Highlights

• Scheduler Integration: Integrates gocron to schedule periodic polling of devbuild status, improving automation.
• Code Refactoring: Refactors devbuild polling logic into separate functions (loopPollDevbuildStatus and pollDevbuildStatus) for better modularity and readability.
• Context Updates: Updates NewRootForMessage to accept and initialize the gocron scheduler, making it available throughout the application.

Changelog

Click here to see the changelog

• chatops-lark/cmd/server/main.go
  • Imported the gocron package.
  • Initialized the gocron scheduler.
  • Passed the scheduler instance to NewRootForMessage.
  • Started the scheduler and ensured it stops if the WebSocket client fails to start.
• chatops-lark/go.mod
  • Added github.com/jasonlvhit/gocron v0.0.1 as a dependency.
• chatops-lark/go.sum
  • Added checksums for gocron and its dependencies.
• chatops-lark/pkg/events/handler/devbuild.go
  • Added constants for context keys related to the devbuild polling scheduler and task.
• chatops-lark/pkg/events/handler/devbuild_poll.go
  • Added slices and time imports.
  • Introduced loopPollInterval constant for the polling interval.
  • Refactored runCommandDevbuildPoll to use pollDevbuildStatus and renderDevbuildStatusForLark.
  • Created loopPollDevbuildStatus to poll until a final status is reached.
  • Created pollDevbuildStatus to handle the actual API call.
• chatops-lark/pkg/events/handler/devbuild_trigger.go
  • Imported the gocron package.
  • Scheduled a polling task using the gocron scheduler in runCommandDevbuildTrigger.
• chatops-lark/pkg/events/handler/root.go
  • Imported the gocron package.
  • Modified the rootHandler struct to include a cronScheduler field.
  • Modified NewRootForMessage to accept a cronScheduler parameter and initialize the cronScheduler field in rootHandler.
  • Added AsyncNotifyHandler to CommandConfig struct.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

A scheduler's gentle hand,
Tasks it starts across the land,
Polling 'til it's done,
Status found and won,
Devbuild's fate, we understand.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The pull request introduces a gocron scheduler to handle periodic polling of devbuild status, updates NewRootForMessage to accept and initialize the scheduler, modifies runCommandDevbuildTrigger to schedule polling tasks, and refactors devbuild polling logic into separate functions. Overall, the changes seem well-structured and address the intended functionality. However, there are a few areas that could be improved for clarity, error handling, and resource management.

Summary of Findings

• Context Value Usage: In runCommandDevbuildTrigger, the context values are retrieved using empty strings as keys, which is incorrect and will lead to unexpected behavior. The correct context keys (ctxKeyDevbuildPollScheduler and ctxKeyDevbuildPollTask) should be used to retrieve the scheduler and polling function.
• Scheduler Start/Stop: The scheduler's Start method returns a channel to stop the scheduler, which is used when the WebSocket client fails to start. However, there's no mechanism to stop the scheduler if the application exits normally. This could lead to resource leaks.
• Error Handling in Main: The error handling in main.go could be improved by ensuring that the cron scheduler is stopped before exiting the application, even if the WebSocket client starts successfully.

Merge Readiness

The pull request requires changes before merging. The context value usage in runCommandDevbuildTrigger must be corrected to ensure the scheduler and polling function are properly retrieved. Additionally, the scheduler's lifecycle should be managed more carefully to prevent resource leaks. I am unable to approve this pull request, and recommend that it not be merged until these issues are addressed, and that others review and approve this code before merging.

[gemini-code-assist]

The context values are being retrieved using empty strings as keys, which is incorrect. Use the constants defined in devbuild.go (ctxKeyDevbuildPollScheduler and ctxKeyDevbuildPollTask) to retrieve the scheduler and polling function. This is a critical error as it will cause the application to not work as expected.

Suggested change

	pollScheduler := ctx.Value("").(*gocron.Scheduler)
	pollFunc := ctx.Value("").(func(string))
	pollScheduler := ctx.Value(ctxKeyDevbuildPollScheduler).(*gocron.Scheduler)
	pollFunc := ctx.Value(ctxKeyDevbuildPollTask).(func(string))

[gemini-code-assist]

Consider adding a defer statement to ensure the scheduler is stopped when the main function exits, even if the WebSocket client starts successfully. This will prevent potential resource leaks if the application exits normally.

Suggested change

	if err := consumerCli.Start(context.Background()); err != nil {
	csStop <- true
	log.Fatal().Err(err).Msg("run failed for Lark WebSocket client")
	}
	if err := consumerCli.Start(context.Background()); err != nil {
	csStop <- true
	log.Fatal().Err(err).Msg("run failed for Lark WebSocket client")
	} else {
	defer func() {
	csStop <- true // Ensure scheduler is stopped on normal exit
	}()
	}

[ti-chi-bot]

@gemini-code-assist[bot]: changing LGTM is restricted to collaborators

In response to this:

Code Review

The pull request introduces a gocron scheduler to handle periodic polling of devbuild status, updates NewRootForMessage to accept and initialize the scheduler, modifies runCommandDevbuildTrigger to schedule polling tasks, and refactors devbuild polling logic into separate functions. Overall, the changes seem well-structured and address the intended functionality. However, there are a few areas that could be improved for clarity, error handling, and resource management.

Summary of Findings

• Context Value Usage: In runCommandDevbuildTrigger, the context values are retrieved using empty strings as keys, which is incorrect and will lead to unexpected behavior. The correct context keys (ctxKeyDevbuildPollScheduler and ctxKeyDevbuildPollTask) should be used to retrieve the scheduler and polling function.
• Scheduler Start/Stop: The scheduler's Start method returns a channel to stop the scheduler, which is used when the WebSocket client fails to start. However, there's no mechanism to stop the scheduler if the application exits normally. This could lead to resource leaks.
• Error Handling in Main: The error handling in main.go could be improved by ensuring that the cron scheduler is stopped before exiting the application, even if the WebSocket client starts successfully.

Merge Readiness

The pull request requires changes before merging. The context value usage in runCommandDevbuildTrigger must be corrected to ensure the scheduler and polling function are properly retrieved. Additionally, the scheduler's lifecycle should be managed more carefully to prevent resource leaks. I am unable to approve this pull request, and recommend that it not be merged until these issues are addressed, and that others review and approve this code before merging.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.