-
Notifications
You must be signed in to change notification settings - Fork 62
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