-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Search Documents] Migrating search-documents to the new recorder #20125
Conversation
What do you think about doing al recorder start and setup inside createClient? Maybe it can take the recorder as parameter |
Sounds good |
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.
😃 Awesome!
import { delay, serviceVersions } from "../../../src/serviceUtils"; | ||
import { versionsToTest } from "@azure/test-utils"; | ||
|
||
const TEST_INDEX_NAME = isLiveMode() ? createRandomIndexName() : "hotel-live-test1"; |
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.
Could you please expand why this is removed? The CreateRandonIndexName method should be called only for live mode. But now it seems it is getting called even for the playback mode which will cause issues
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.
< Discussed offline 😎 >
Spoke with Harsha offline. Changes look good. Approving it |
Packages impacted by this PR
search-documents
Issues associated with this PR
#19859
Describe the problem that is addressed by this PR
Migrating search-documents to the new recorder
What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
N/A
Are there test cases added in this PR? (If not, why?)
N/A
Provide a list of related PRs (if any)
N/A
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Checklists