-
Notifications
You must be signed in to change notification settings - Fork 58
Add missing ability to configure QueryClientSettings #512
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
Conversation
41caeac
to
8c43592
Compare
🌋 Here are results of SLO test for Python SDK over Query Service: |
🌋 Here are results of SLO test for Python SDK over Table Service: |
8c3e524
to
f0a84f6
Compare
f0a84f6
to
e757eef
Compare
return settings | ||
|
||
|
||
test_td = timedelta(microseconds=-100) |
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.
why do you need 100 microseconds?
can you use one second or one millisecond?
|
||
test_td = timedelta(microseconds=-100) | ||
test_now = datetime.utcnow() | ||
test_today = test_now.date() |
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.
what about use fixed date/time instead of now?
|
||
|
||
params = pytest.mark.parametrize( | ||
"value,ydb_type,casted_result,uncasted_type", |
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.
casted_result and uncasted_type are difficult to underdstand.
what about: raw result and native_result?
and always compare values
self, | ||
driver: common_utils.SupportedDriverType, | ||
size: int = 100, | ||
query_client_settings: Optional[QueryClientSettings] = None, |
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.
what about explicit named paremeter for the settings?
self, | ||
driver: common_utils.SupportedDriverType, | ||
size: int = 100, | ||
query_client_settings: Optional[QueryClientSettings] = None, |
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.
and here (named parameter)
db6de2e
to
f908698
Compare
f908698
to
54bf667
Compare
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Other information