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

Draft: New Redis extension #285

Merged
merged 106 commits into from
Mar 21, 2022
Merged

Draft: New Redis extension #285

merged 106 commits into from
Mar 21, 2022

Conversation

hanspagel
Copy link
Contributor

@hanspagel hanspagel commented Jan 15, 2022

This PR has a rewrite of the Redis extension. On my way, I wanted to and had to refactor a lot of core things, too. Eventually, it became a super huge PR. I’ve created and merged a new PR #288 for the unrelated changes.

I still see a few areas I’d like to improve.

Changes

  • Rewrite of the Redis extension
  • Removes the y-redis dependency
  • Syncs Awareness states between instances

Tasks

  • Get the test suite running stable

Known issues

  • getConnectionCount() returns the number of connections to the current instance
  • getDocumentsCount() returns the number of connections to the current instance
  • closeConnections() closes connections on the current instance

@hanspagel hanspagel mentioned this pull request Jan 15, 2022
7 tasks
@hanspagel
Copy link
Contributor Author

hanspagel commented Jan 17, 2022

I migrated more tests over to AVA, to test if it really works with more tests … and turns out it’s flaky again.

avajs/ava#2947

Reminder to myself: Never work with Node.js again. People are nice.

@gaelbonjour
Copy link
Collaborator

Hi :)
Would love to see this rework of the redis extension in action for the awareness being missing on the current implem is a huge blocker for our use case.

@hanspagel
Copy link
Contributor Author

Time to merge. :-)

@hanspagel hanspagel merged commit eed75c6 into main Mar 21, 2022
@hanspagel hanspagel deleted the feature/new-redis-extension branch March 21, 2022 14:24
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.

3 participants