FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

allow manual redis connection by billyen2012 · Pull Request #708 · bee-queue/bee-queue · GitHub

allow manual redis connection - #708

Merged
compwright merged 1 commit into
bee-queue:masterfrom
billyen2012:allow-manual-connection
Nov 2, 2023
Merged

allow manual redis connection#708
compwright merged 1 commit into
bee-queue:masterfrom
billyen2012:allow-manual-connection

Conversation

Copy link
Copy Markdown
Contributor

Hey, this is a super awesome queue library.

I'm working on a next.js project and found that whenever I try to build the project, the build process will failed because Queue is try to connect to the redis host for some reasons.

Not sure if it is because the logic of connection to redis host is in the constructor of the Queue, but be able manually connect to redis seems to solve the problem.

So here is the pull request basically allowing user to manual connect to redis by calling queue.connect() after Queue instance is created.

  • test is added and README is updated accordingly;
  • there is also minor code refactor.

compwright left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you for this contribution! I think it would be a bit more user-friendly to call the new option autoConnect and default it to true. If you don't want auto connection then you would manually set the option to false and call queue.connect(). Would you make this change for me?

Copy link
Copy Markdown
Collaborator

@billyen2012 also please note the failing CI checks. You need to edit the commit message for all commits on this branch to comply with this standard: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

billyen2012 force-pushed the allow-manual-connection branch 2 times, most recently from b55330b to b94b43b Compare October 27, 2023 05:23
billyen2012 force-pushed the allow-manual-connection branch from b94b43b to 1accf5d Compare October 27, 2023 05:56

Copy link
Copy Markdown
Contributor Author

Thank you for this contribution! I think it would be a bit more user-friendly to call the new option autoConnect and default it to true. If you don't want auto connection then you would manually set the option to false and call queue.connect(). Would you make this change for me?

Suggested change has been applied

Copy link
Copy Markdown
Contributor Author

@billyen2012 also please note the failing CI checks. You need to edit the commit message for all commits on this branch to comply with this standard: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

Opps, I was unaware that I suppose to use npm run ci for all the test.

should be okay now

I also notice that sometimes the test will failed because when all the test suite are executed together, it will just make redis dropping connection. Not sure if it was just the issue of my redis, but I will suggest the following change to make the test more stable. (this is not in this PR)

compwright merged commit 425fb89 into bee-queue:master Nov 2, 2023
beequeueci pushed a commit that referenced this pull request Nov 2, 2023
## [1.6.0](v1.5.0...v1.6.0) (2023-11-02)

### Features

* **queue:** allow manual connection ([#708](#708)) ([425fb89](425fb89))

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL