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.

Sign up for GitHub

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

Jump to bottom

0.9.0 changes #35

Merged
merged 218 commits into from
Jul 24, 2024
Merged

0.9.0 changes #35

merged 218 commits into from
Jul 24, 2024

Conversation

DomPeliniAerospike
Copy link
Collaborator

No description provided.

DomPeliniAerospike and others added 30 commits May 23, 2024 11:30
@DomPeliniAerospike
Added basic CI/CD
7470d4c
@DomPeliniAerospike
Update integration_test.yml
305e8ce
@DomPeliniAerospike
Update integration_test.yml
892a9db
@DomPeliniAerospike
Update integration_test.yml
baaf967
@DomPeliniAerospike
Testing git-crypt
de1b66d
@DomPeliniAerospike
Fixed integration test to work with new features.conf
1c1fa7d
@DomPeliniAerospike
typo
6d5fcab
@DomPeliniAerospike
Update integration_test.yml
388164f
@DomPeliniAerospike
Update integration_test.yml
073c5d8
@DomPeliniAerospike
testing encryption of file.
10c190a
@DomPeliniAerospike
Fixing features.conf
ecca7ed
@DomPeliniAerospike
Initial commit
6f6dcb4
@DomPeliniAerospike
Enabled client to work with either Auth+TLS or plain text
9b90d59
@DomPeliniAerospike
Finializing RBAC Functionality and tests
4324c86
@DomPeliniAerospike
MTLS dirty commit
8ba080f 
Commit for Rahul to look through
@DomPeliniAerospike
Update README.md
e962e27
@DomPeliniAerospike
Merge pull request #25 from aerospike/CI/CD-init
f064588 
Ci/cd init
@DomPeliniAerospike
Added jwt to requirements.txt
8d6a884
@DomPeliniAerospike
Update requirements.txt
57e8082
@DomPeliniAerospike
Fixed jwt install
2a96ccd
@DomPeliniAerospike
Update requirements.txt
f30771c
@DomPeliniAerospike
Update integration_test.yml
290e0bb
@DomPeliniAerospike
Update requirements.txt
0f8be02
@DomPeliniAerospike
Update integration_test.yml
a236810
@DomPeliniAerospike
Update integration_test.yml
f177485
@DomPeliniAerospike
Update integration_test.yml
d72c10a
@DomPeliniAerospike
Update integration_test.yml
5417615
@DomPeliniAerospike
Update integration_test.yml
2039ccf
@DomPeliniAerospike
Update integration_test.yml
207122e
@DomPeliniAerospike
Update integration_test.yml
af213cf
jdogmcsteezy
jdogmcsteezy suggested changes Jul 23, 2024
View reviewed changes
Copy link
Contributor

@jdogmcsteezy jdogmcsteezy left a comment

Choose a reason for hiding this comment

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

Looks good! Large PRs come with a lot of comments. Keep in mind that I tried to not repeat myself when a comment could apply to both async and sync clients

pyproject.toml Show resolved Hide resolved
src/aerospike_vector_search/admin.py Show resolved Hide resolved
src/aerospike_vector_search/admin.py Show resolved Hide resolved
src/aerospike_vector_search/admin.py Outdated Show resolved Hide resolved
src/aerospike_vector_search/admin.py Outdated Show resolved Hide resolved
tests/aerospike-vector-search.yml Show resolved Hide resolved
tests/assets/aerospike-proximus.yml Outdated Show resolved Hide resolved
src/aerospike_vector_search/aio/client.py Outdated Show resolved Hide resolved
src/aerospike_vector_search/aio/client.py Outdated Show resolved Hide resolved
src/aerospike_vector_search/aio/client.py Show resolved Hide resolved
@DomPeliniAerospike
Copy link
Collaborator Author

Responded to all of Jesse's comments, requesting re-review

@jdogmcsteezy
Copy link
Contributor

responded

jdogmcsteezy
jdogmcsteezy suggested changes Jul 23, 2024
View reviewed changes
src/aerospike_vector_search/admin.py Outdated Show resolved Hide resolved
src/aerospike_vector_search/internal/channel_provider.py Show resolved Hide resolved
src/aerospike_vector_search/admin.py Outdated Show resolved Hide resolved
src/aerospike_vector_search/internal/channel_provider.py Outdated Show resolved Hide resolved
src/aerospike_vector_search/shared/base_channel_provider.py Outdated Show resolved Hide resolved
src/aerospike_vector_search/types.py Show resolved Hide resolved
src/aerospike_vector_search/types.py Outdated Show resolved Hide resolved
jdogmcsteezy
jdogmcsteezy reviewed Jul 23, 2024
View reviewed changes
Copy link
Contributor

@jdogmcsteezy jdogmcsteezy left a comment

Choose a reason for hiding this comment

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

I don't see any changes to the returned value. Did it already return a dict?

dwelch-spike
dwelch-spike reviewed Jul 23, 2024
View reviewed changes
Copy link
Contributor

@dwelch-spike dwelch-spike left a comment

Choose a reason for hiding this comment

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

Non-blocking: Review of docstrings. Looks good, some comments and nitpicks

docs/sync.rst Show resolved Hide resolved
src/aerospike_vector_search/admin.py Show resolved Hide resolved
src/aerospike_vector_search/admin.py Show resolved Hide resolved
src/aerospike_vector_search/admin.py Show resolved Hide resolved
src/aerospike_vector_search/admin.py Show resolved Hide resolved
src/aerospike_vector_search/client.py Show resolved Hide resolved
src/aerospike_vector_search/client.py Show resolved Hide resolved
src/aerospike_vector_search/client.py Show resolved Hide resolved
src/aerospike_vector_search/client.py Show resolved Hide resolved
src/aerospike_vector_search/client.py Show resolved Hide resolved
@DomPeliniAerospike
Copy link
Collaborator Author

I will adresss Dylan's review comments after release

@DomPeliniAerospike DomPeliniAerospike merged commit 1a4655b into dev Jul 24, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdogmcsteezy jdogmcsteezy jdogmcsteezy requested changes

@dwelch-spike dwelch-spike dwelch-spike left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DomPeliniAerospike @jdogmcsteezy @dwelch-spike