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

Feature: redis storage #1396

Merged
merged 20 commits into from Nov 11, 2023
Merged

Feature: redis storage #1396

merged 20 commits into from Nov 11, 2023

Conversation

gurugio
Copy link
Contributor

@gurugio gurugio commented Sep 2, 2023

I'm working for Redis storage.
There are many patches at the moment. I will rebase them into one patch later.
Please check the latest code.

@ever0de ever0de added the enhancement New feature or request label Sep 2, 2023
@gurugio gurugio force-pushed the feature/redis-storage branch 8 times, most recently from 5ebeff7 to 5ed4765 Compare September 23, 2023 06:35
@coveralls
Copy link

coveralls commented Sep 23, 2023

Pull Request Test Coverage Report for Build 6832207731

  • 855 of 1040 (82.21%) changed or added relevant lines in 9 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.3%) to 98.26%

Changes Missing Coverage Covered Lines Changed/Added Lines %
storages/redis-storage/tests/redis_reconnect.rs 55 58 94.83%
storages/redis-storage/tests/redis_table.rs 160 167 95.81%
storages/redis-storage/src/metadata.rs 25 35 71.43%
storages/redis-storage/tests/redis_store.rs 161 171 94.15%
storages/redis-storage/src/index.rs 15 26 57.69%
storages/redis-storage/src/alter_table.rs 139 179 77.65%
storages/redis-storage/src/lib.rs 254 358 70.95%
Totals Coverage Status
Change from base Build 6753424032: -0.3%
Covered Lines: 51447
Relevant Lines: 52358

💛 - Coveralls

@gurugio gurugio force-pushed the feature/redis-storage branch 2 times, most recently from 6a126dc to 93de0f6 Compare September 24, 2023 11:00
@gurugio gurugio marked this pull request as ready for review September 27, 2023 15:33
@gurugio gurugio force-pushed the feature/redis-storage branch 2 times, most recently from b86f91d to c059dc9 Compare October 4, 2023 14:00
@panarch panarch self-requested a review October 29, 2023 06:38
storages/redis-storage/src/alter_table.rs Outdated Show resolved Hide resolved
storages/redis-storage/src/lib.rs Outdated Show resolved Hide resolved
storages/redis-storage/tests/redis-storage.toml Outdated Show resolved Hide resolved
storages/redis-storage/tests/redis_reconnect.rs Outdated Show resolved Hide resolved
storages/redis-storage/tests/redis_store.rs Outdated Show resolved Hide resolved
storages/redis-storage/tests/redis_store.rs Outdated Show resolved Hide resolved
storages/redis-storage/tests/redis_table.rs Outdated Show resolved Hide resolved
storages/redis-storage/tests/redis_table.rs Outdated Show resolved Hide resolved
storages/redis-storage/tests/redis_table.rs Outdated Show resolved Hide resolved
Store Redis server address and port number in an extra file.
Each test reads the file when connecting the Redis server
@panarch panarch self-requested a review November 5, 2023 13:55
[dev-dependencies]
test-suite.workspace = true
tokio = { version = "1", features = ["rt", "macros"] }
futures = "0.3"
Copy link
Member

Choose a reason for hiding this comment

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

futures is already declared in [dependencies] so we can remove it here.

serde_json = "1.0.105"
chrono = { version = "=0.4.31", features = ["serde", "wasmbind"] }
futures = "0.3"
toml = "0.8.6"
Copy link
Member

Choose a reason for hiding this comment

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

toml is currently only used in tests/ so we can declare it in dev-dependencies rather than dependencies.

serde = { version = "1", features = ["derive"] }
redis = "0.23.3"
serde_json = "1.0.105"
chrono = { version = "=0.4.31", features = ["serde", "wasmbind"] }
Copy link
Member

Choose a reason for hiding this comment

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

is there any reason to specify =0.4.31? it would be enough to use 0.4.31.

@panarch panarch self-requested a review November 11, 2023 03:44
Copy link
Member

@panarch panarch left a comment

Choose a reason for hiding this comment

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

Great work 👍 Looks all nice. Thanks a lot for the contribution 👍 👍 Let's ship this to the main!

@panarch panarch merged commit 8a2cb5b into gluesql:main Nov 11, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants