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.

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

Adding the QueryExecutor implementation #19515

Merged
merged 5 commits into from
Mar 5, 2021
Merged

Adding the QueryExecutor implementation #19515

merged 5 commits into from
Mar 5, 2021

Conversation

amarathavale
Copy link
Contributor

Changes included in this PR

  1. Added the QueryExecutor class that facilitates sql query, using the SDK's queryItems method. This implementation is a copy-paste of what we are using.
  2. Added the query interface method to Accessor. The QueryExecutor/query mechanism has not been integrated into the CTL workload yet (to limit the number of files changed in this request)
  3. Refactored the resource management after the back-and-forth on what should be managed by the test and handled by the CTL workload. For local testing, the database needs to be created/managed by the test itself. Hence, added the Database management part of the resource setup.
  4. Subsequently, removed the -manageResource flag from the shell script used by the Docker container.

@ghost ghost added Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Mar 1, 2021
@ghost
Copy link

ghost commented Mar 1, 2021

Thank you for your contribution amarathavale! We will review the pull request and get back to you soon.

Copy link
Contributor

@moderakh moderakh left a comment

Choose a reason for hiding this comment

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

please wait for @simplynaveen20 review and sign off.

Copy link
Member

@simplynaveen20 simplynaveen20 left a comment

Choose a reason for hiding this comment

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

Apart from minor comments , this looks good to me. If you want to send another iteration , please address the comments other wise we can merge, your choice. On image upload to azure registry , we are good to upload it in current state

@simplynaveen20 simplynaveen20 merged commit 7786a39 into Azure:master Mar 5, 2021
@amarathavale amarathavale deleted the linkedin/query-ctl-workload branch March 7, 2021 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants