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

Search: Skill Versions #20431

Merged
merged 17 commits into from
Sep 3, 2021
Merged

Search: Skill Versions #20431

merged 17 commits into from
Sep 3, 2021

Conversation

tjprescott
Copy link
Member

@tjprescott tjprescott commented Aug 26, 2021

Closes #20432.

Adds SentimentSkillVersion and EntityRecognitionSkillVersion, and a customized model for SentimentSkill and EntityRecognitionSkill that uses this version to create the correct model.

  • Add tests

@tjprescott tjprescott requested a review from xiangyan99 August 26, 2021 17:01
@ghost ghost added the Search label Aug 26, 2021
Copy link
Member

@xiangyan99 xiangyan99 left a comment

Choose a reason for hiding this comment

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

Seems like we missed the de-serialization part.

SearchIndexerSkillset.skills is [SearchIndexerSkill].

It seems to me we also need to customize SearchIndexerSkillset so if its skills contains EntityRecognitionSkill/SentimentSkill. We need to map them into our fancy Skills.

@tjprescott tjprescott marked this pull request as ready for review August 31, 2021 17:03
Copy link
Member

@xiangyan99 xiangyan99 left a comment

Choose a reason for hiding this comment

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

nit: I think this change should not impact any network request/response. So no need to re-record the tests. Of course, there is no harm to do so.

@tjprescott
Copy link
Member Author

I had to re-record the tests because I needed to change the skillset tests... and then re-recorded all to ensure I didn't accidentally break anything.

@tjprescott
Copy link
Member Author

@annatisch @xiangyan99 I created an issue to track the client-side validation aspects: #20526

@tjprescott tjprescott merged commit d98e42f into Azure:main Sep 3, 2021
@tjprescott tjprescott deleted the tjprescott/SearchSkillVersion branch September 3, 2021 19:41
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Sep 7, 2021
…into switch_to_protocol

* 'main' of https://github.com/Azure/azure-sdk-for-python: (53 commits)
  Smoke test package verification (Azure#20547)
  Update CHANGELOG.md (Azure#20569)
  updating codeowners (Azure#20570)
  Simplify recorded OnBehalfOfCredential tests (Azure#20568)
  Search: update release date (Azure#20564)
  [Exporter] Support redirect response in exporter (Azure#20489)
  [AutoRelease] t2-synapse-2021-09-06-18774 (Azure#20552)
  [AutoRelease] t2-logz-2021-08-26-01773 (Azure#20426)
  [AutoRelease] t2-relay-2021-09-03-32777 (Azure#20530)
  [AutoRelease] t2-resource-2021-09-03-61345 (Azure#20527)
  [AutoRelease] t2-servicefabric-2021-09-02-41879 (Azure#20512)
  [Keyvault] Remove exception message parsing from samples in keys, certificates and secrets (Azure#20540)
  [ACR] Update cloud configuration API (Azure#20464)
  Get rid of generated code (Azure#20536)
  Search: Skill Versions (Azure#20431)
  Check fd is reg file or symlink in get_length before using st_size. (Azure#19725)
  Add OnBehalfOfCredential (Azure#20451)
  Fix incorrect parsing of message from Exception (Azure#20534)
  [Release sdk status] swagger repo default branch changes to `main` (Azure#20529)
  [AutoRelease] t2-rdbms-2021-09-02-91864 (Azure#20513)
  ...
iscai-msft pushed a commit that referenced this pull request Sep 29, 2021
* Initial commit.

* Add docstrings for skill_version

* Ensure SearchIndexerSkillset can convert between custom and generated models.

* Update tests.

* Fix async skillsets and tests.

* Add client-side validation and tests for model properties.

* Re-record tests.

* Fix pylint errors.

* Rerecord tests. Fix linter errors.

* Fix ordering on old Python version.

* Add changelog entry.

* Fix linter issues. Make enums NOT case insensitive due to pylint failure.

* Remove client-side validation. Update models.

* Update test and re-record skillset tests.

* Change :param annotations to :ivar for custom models.

* Rename SearchField normalizer to normalizer_name.

* Restore :param annotation for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search: Add Skill Versions
3 participants