-
Notifications
You must be signed in to change notification settings - Fork 9
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
Package review PR #1
base: test
Are you sure you want to change the base?
Changes from 46 commits
5237a10
a96c08a
be97a25
5ebc1cb
8557b55
1729520
be80d9d
ed4d0f9
2db0e4b
778c219
b11970e
bdf5f0d
8f5e3fc
5545680
42d015c
481ecac
ff60817
1c1b6ab
d00fa99
053d5e9
fbdfb5b
a2343b7
753575a
3f85cae
159c938
bebf72a
ef4e215
6ca68c5
b7504e9
3ebb15b
ec72262
2816474
3c285d5
ffb722f
5b0df15
c66243d
0d2ad8a
85e20fd
0b9a90d
a180f42
736a48a
6d0a4b6
dee8a7e
145f9b2
282599d
687bec7
f299216
4d38649
fde4d37
f5aa39e
57a7901
12031b2
31d21c0
b301baa
a33e0e3
7c25468
34d6fb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
name: ci | ||
|
||
on: | ||
push: | ||
pull_request: | ||
|
||
env: | ||
CARGO_TERM_COLOR: always | ||
VALKEY_REPO_URL: https://github.com/valkey-io/valkey.git | ||
|
||
jobs: | ||
build-ubuntu-latest: | ||
runs-on: ubuntu-latest | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
server_version: ['unstable', '8.0.0'] | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- name: Set the server verison for python integeration tests | ||
run: echo "SERVER_VERSION=${{ matrix.server_version }}" >> $GITHUB_ENV | ||
- name: Run cargo and clippy format check | ||
run: | | ||
cargo fmt --check | ||
cargo clippy --profile release --all-targets -- -D clippy::all | ||
- name: Release Build | ||
run: cargo build --all --all-targets --release | ||
- name: Run unit tests | ||
run: cargo test --features enable-system-alloc | ||
- name: Make valkey-server binary | ||
run: | | ||
mkdir -p "tests/.build/binaries/${{ matrix.server_version }}" | ||
cd tests/.build | ||
git clone "${{ env.VALKEY_REPO_URL }}" | ||
cd valkey | ||
git checkout ${{ matrix.server_version }} | ||
make | ||
cp src/valkey-server ../binaries/${{ matrix.server_version }}/ | ||
- name: Set up Python | ||
uses: actions/setup-python@v3 | ||
with: | ||
python-version: '3.8' | ||
- name: Install dependencies | ||
run: | | ||
python -m pip install --upgrade pip | ||
pip install -r requirements.txt | ||
- name: Update module path | ||
run: echo "MODULE_PATH=$(realpath target/release/libvalkey_bloom.so)" >> $GITHUB_ENV | ||
- name: Run integration tests | ||
run: python -m pytest --cache-clear -v "tests/" | ||
|
||
build-macos-latest: | ||
runs-on: macos-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- name: Run cargo and clippy format check | ||
run: | | ||
cargo fmt --check | ||
cargo clippy --profile release --all-targets -- -D clippy::all | ||
- name: Release Build | ||
run: cargo build --all --all-targets --release | ||
- name: Run unit tests | ||
run: cargo test --features enable-system-alloc | ||
|
||
asan-build: | ||
runs-on: ubuntu-latest | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
server_version: ['unstable', '8.0.0'] | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- name: Set the server verison for python integeration tests | ||
run: echo "SERVER_VERSION=${{ matrix.server_version }}" >> $GITHUB_ENV | ||
- name: Run cargo and clippy format check | ||
run: | | ||
cargo fmt --check | ||
cargo clippy --profile release --all-targets -- -D clippy::all | ||
- name: Release Build | ||
run: cargo build --all --all-targets --release | ||
- name: Run unit tests | ||
run: cargo test --features enable-system-alloc | ||
- name: Make Valkey-server binary with asan | ||
run: | | ||
mkdir -p "tests/.build/binaries/${{ matrix.server_version }}" | ||
cd tests/.build | ||
git clone "${{ env.VALKEY_REPO_URL }}" | ||
cd valkey | ||
git checkout ${{ matrix.server_version }} | ||
make SANITIZER=address SERVER_CFLAGS='-Werror' BUILD_TLS=module | ||
cp src/valkey-server ../binaries/${{ matrix.server_version }}/ | ||
- name: Set up Python | ||
uses: actions/setup-python@v3 | ||
with: | ||
python-version: '3.8' | ||
- name: Install dependencies | ||
run: | | ||
python -m pip install --upgrade pip | ||
pip install -r requirements.txt | ||
- name: Update module path | ||
run: echo "MODULE_PATH=$(realpath target/release/libvalkey_bloom.so)" >> $GITHUB_ENV | ||
- name: Run integration tests | ||
run: python -m pytest --cache-clear -v "tests/" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
Cargo.lock | ||
target | ||
tests/.build | ||
__pycache__ | ||
test-data | ||
.attach_pid* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
[package] | ||
name = "valkey-bloom" | ||
authors = ["Karthik Subbarao"] | ||
version = "0.1.0" | ||
edition = "2021" | ||
license = "BSD-3-Clause" | ||
repository = "https://github.com/valkey-io/valkey-bloom" | ||
readme = "README.md" | ||
description = "A bloom filter module for Valkey" | ||
homepage = "https://github.com/valkey-io/valkey-bloom" | ||
|
||
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html | ||
|
||
[dependencies] | ||
valkey-module = "0.1.2" | ||
bloomfilter = "1.0.13" | ||
lazy_static = "1.4.0" | ||
libc = "0.2" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need libc? |
||
|
||
[dev-dependencies] | ||
rand = "0.8" | ||
|
||
[lib] | ||
crate-type = ["cdylib"] | ||
name = "valkey_bloom" | ||
|
||
[profile.dev] | ||
opt-level = 0 | ||
debug = 2 | ||
debug-assertions = true | ||
|
||
[features] | ||
enable-system-alloc = ["valkey-module/enable-system-alloc"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the intention with this configuration? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,116 @@ | ||
# valkey-bloom | ||
# valkey-bloom | ||
|
||
Valkey-Bloom (BSD-3-Clause) is a Rust Valkey-Module which brings a native and space efficient probabilistic Module data type to Valkey. With this, users can create filters (space-efficient probabilistic Module data type) to add elements, perform “check” operation to test whether an element exists, auto scale their filters, perform RDB Save and load operations, etc. | ||
|
||
Valkey-Bloom is built using bloomfilter::Bloom (https://crates.io/crates/bloomfilter which has a BSD-2-Clause license). | ||
|
||
It is compatible with the BloomFilter (BF.*) command APIs of the ReBloom Module from Redis Ltd. | ||
|
||
The following commands are supported. | ||
``` | ||
BF.EXISTS | ||
BF.ADD | ||
BF.MEXISTS | ||
BF.MADD | ||
BF.CARD | ||
BF.RESERVE | ||
BF.INFO | ||
BF.INSERT | ||
``` | ||
|
||
Build instructions for Linux. | ||
``` | ||
curl https://sh.rustup.rs -sSf | sh | ||
sudo yum install clang | ||
git clone https://github.com/KarthikSubbarao/valkey-bloom.git | ||
cd valkey-bloom | ||
cargo build --all --all-targets --release | ||
valkey-server --loadmodule ./target/release/libvalkey_bloom.so | ||
``` | ||
|
||
Local development script to build, run format checks, run unit / integration tests, and for cargo release: | ||
``` | ||
# Builds the valkey-server (unstable) for integration testing. | ||
SERVER_VERSION=unstable | ||
./build.sh | ||
# Builds the valkey-server (8.0.0) for integration testing. | ||
SERVER_VERSION=8.0.0 | ||
./build.sh | ||
``` | ||
|
||
Client Usage | ||
``` | ||
<redacted> % ./valkey-cli | ||
127.0.0.1:6379> module list | ||
1) 1) "name" | ||
2) "bloom" | ||
3) "ver" | ||
4) (integer) 1 | ||
5) "path" | ||
6) "./target/release/libvalkey_bloom.so" | ||
7) "args" | ||
8) (empty array) | ||
127.0.0.1:6379> bf.add key item | ||
(integer) 1 | ||
127.0.0.1:6379> bf.exists key item | ||
(integer) 1 | ||
127.0.0.1:6379> bf.exists key item2 | ||
(integer) 0 | ||
127.0.0.1:6379> bf.card key | ||
(integer) 1 | ||
127.0.0.1:6379> bf.reserve key 0.01 10000 | ||
(error) ERR item exists | ||
127.0.0.1:6379> bf.reserve key1 0.01 10000 | ||
OK | ||
127.0.0.1:6379> bf.card key1 | ||
(integer) 0 | ||
127.0.0.1:6379> bf.add key1 item | ||
(integer) 1 | ||
127.0.0.1:6379> bf.card key1 | ||
(integer) 1 | ||
``` | ||
|
||
``` | ||
127.0.0.1:6379> bf.reserve key1 0.01 10000 | ||
OK | ||
127.0.0.1:6379> bf.info key3 | ||
(empty array) | ||
127.0.0.1:6379> bf.info key1 | ||
1) Capacity | ||
2) (integer) 10000 | ||
3) Size | ||
4) (integer) 12198 | ||
5) Number of filters | ||
6) (integer) 1 | ||
7) Number of items inserted | ||
8) (integer) 0 | ||
9) Expansion rate | ||
10) (integer) 2 | ||
``` | ||
|
||
RDB Load, Save and flushall validation | ||
``` | ||
127.0.0.1:6379> info keyspace | ||
# Keyspace | ||
127.0.0.1:6379> bf.add key item | ||
(integer) 1 | ||
127.0.0.1:6379> info keyspace | ||
# Keyspace | ||
db0:keys=1,expires=0,avg_ttl=0 | ||
127.0.0.1:6379> flushall | ||
OK | ||
127.0.0.1:6379> info keyspace | ||
# Keyspace | ||
127.0.0.1:6379> bf.add key item | ||
(integer) 1 | ||
127.0.0.1:6379> bgsave | ||
Background saving started | ||
127.0.0.1:6379> shutdown | ||
not connected> info keyspace // Started up | ||
# Keyspace | ||
db0:keys=1,expires=0,avg_ttl=0 | ||
127.0.0.1:6379> keys * | ||
1) "key" | ||
127.0.0.1:6379> bf.exists key item | ||
(integer) 1 | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
#!/usr/bin/env sh | ||
|
||
# Script to run format checks valkey-bloom module, build it and generate .so files, run unit and integration tests. | ||
|
||
# Exit the script if any command fails | ||
set -e | ||
|
||
SCRIPT_DIR=$(pwd) | ||
echo "Script Directory: $SCRIPT_DIR" | ||
|
||
echo "Running cargo and clippy format checks..." | ||
cargo fmt --check | ||
cargo clippy --profile release --all-targets -- -D clippy::all | ||
|
||
echo "Running cargo build release..." | ||
cargo build --all --all-targets --release | ||
|
||
echo "Running unit tests..." | ||
cargo test --features enable-system-alloc | ||
|
||
# Ensure SERVER_VERSION environment variable is set | ||
if [ -z "$SERVER_VERSION" ]; then | ||
echo "ERROR: SERVER_VERSION environment variable is not set. Defaulting to unstable." | ||
export SERVER_VERSION="unstable" | ||
fi | ||
|
||
if [ "$SERVER_VERSION" != "unstable" ] && [ "$SERVER_VERSION" != "8.0.0" ] ; then | ||
echo "ERROR: Unsupported version - $SERVER_VERSION" | ||
exit 1 | ||
fi | ||
|
||
REPO_URL="https://github.com/valkey-io/valkey.git" | ||
BINARY_PATH="tests/.build/binaries/$SERVER_VERSION/valkey-server" | ||
|
||
if [ -f "$BINARY_PATH" ] && [ -x "$BINARY_PATH" ]; then | ||
echo "valkey-server binary '$BINARY_PATH' found." | ||
else | ||
echo "valkey-server binary '$BINARY_PATH' not found." | ||
mkdir -p "tests/.build/binaries/$SERVER_VERSION" | ||
cd tests/.build | ||
rm -rf valkey | ||
git clone "$REPO_URL" | ||
cd valkey | ||
git checkout "$SERVER_VERSION" | ||
make | ||
cp src/valkey-server ../binaries/$SERVER_VERSION/ | ||
fi | ||
|
||
REQUIREMENTS_FILE="requirements.txt" | ||
|
||
# Check if pip is available | ||
if command -v pip > /dev/null 2>&1; then | ||
echo "Using pip to install packages..." | ||
pip install -r "$SCRIPT_DIR/$REQUIREMENTS_FILE" | ||
# Check if pip3 is available | ||
elif command -v pip3 > /dev/null 2>&1; then | ||
echo "Using pip3 to install packages..." | ||
pip3 install -r "$SCRIPT_DIR/$REQUIREMENTS_FILE" | ||
else | ||
echo "Error: Neither pip nor pip3 is available. Please install Python package installer." | ||
exit 1 | ||
fi | ||
|
||
export MODULE_PATH="$SCRIPT_DIR/target/release/libvalkey_bloom.so" | ||
|
||
echo "Running the integration tests..." | ||
python3 -m pytest --cache-clear -v "$SCRIPT_DIR/tests/" | ||
|
||
echo "Build and Integration Tests succeeded" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
valkey | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this file do? |
||
pytest==4 |
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.
If I am a new developer looking at this file, how do I understand the intent of this file without documentation? Are these configurations obvious? What documentation did you read to write this file?