-
Notifications
You must be signed in to change notification settings - Fork 2
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
Vector client release 0.4.0 #7
Conversation
Added logging in a few more areas Added to_pb2 function Removed type_pb2 usage from every function Added asynchronous context manager support for both clients reorder _get_key Tested each parameter in each API.
changed labels to index_meta_data and vector_bin_name to vector_field
Added asyncio.sleep rather than time.sleep Added index_stub to wait functions rather than using get_status Reduced wait for creation and deletion to 0.1 seconds
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.
Generally looks okay, but a few comments:
- proto files: These are shared between clients, how are you going to keep them in sync? Should this be submoduled out?
- Python 3.8 goes end of life this year - perhaps you should move the min Python to 3.9
- main README file needs to be changed - PyPi upload instructions should be removed if this is to be customer facing
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.
I mostly looked at the recent commits because I already reviewed this PR. A couple nit picks but otherwise LGTM
sys.path.insert(0, os.path.abspath('../src/aerospike_vector')) | ||
import sphinx.ext.autodoc | ||
|
||
project = 'D' |
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.
'D', is that right? I am unfamiliar with sphinx
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.
Yeah this was just a place holder while I was getting the hang of the python API docs.
Will fix this with the (next) docs PR.
You can adapt this file completely to your liking, but it should at least | ||
contain the root `toctree` directive. | ||
|
||
Welcome to D's documentation! SCOBEDY BABALOUIE |
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.
D again! :)
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.
I did not need to write this, will fix this soon lol.
@justinlee-aerospike, submoduling is a good idea. I am drawing a blank on what other options we have other than simply copying them over |
Co-authored-by: Jesse S <jschmidt@aerospike.com>
Submoduling needs to happen for the future, probably not a priority for this release. |
VEC-94
VEC-95
VEC-96
VEC-98
VEC-103
VEC-107
VEC-108
VEC-109
VEC-123