Skip to content

Conversation

@jeji1101
Copy link
Contributor

@jeji1101 jeji1101 commented Jul 10, 2018

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
  • If applicable, the PR references the bug/issue that it fixes.
  • Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

@AutorestCI
Copy link

AutorestCI commented Jul 10, 2018

Automation for azure-sdk-for-python

Nothing to generate for azure-sdk-for-python

@AutorestCI
Copy link

AutorestCI commented Jul 10, 2018

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

@AutorestCI
Copy link

AutorestCI commented Jul 10, 2018

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Jul 10, 2018

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

@AutorestCI
Copy link

AutorestCI commented Jul 10, 2018

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@jeji1101 jeji1101 changed the title Jeji/addresscomments Modify Cognitive Search Swagger Jul 10, 2018
"url": "https://docs.microsoft.com/azure/search/cognitive-search-tutorial-blob"
},
"description": "A list of Cognitive Search skills."
"description": "A list of cognitive skills."
Copy link
Contributor

Choose a reason for hiding this comment

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

is this the right description?
only indexers have datasource, skillset and index names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Synced up with Victor offline on this.

"context": {
"type": "string",
"description": "The context of the skill."
"description": "Represents the level at which operations take place, such as the document root or document path (for example, document/sentiment or document/content)."
Copy link
Contributor

Choose a reason for hiding this comment

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

document root or document content (for example, /document or /document/content)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

"name": {
"type": "string",
"description": "The name of the output."
"description": "The name path of the output."
Copy link
Contributor

Choose a reason for hiding this comment

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

The name of the output defined by the skill.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

"targetName"],
"description": "output field mapping for a skill."
"required": [
"name", "targetName"
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe targetName is optional and defaults to name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

"externalDocs": {
"url": "https://docs.microsoft.com/rest/api/searchservice/naming-rules"
},
"description": "Output field mapping for a skill."
Copy link
Contributor

Choose a reason for hiding this comment

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

Name of the skill output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Synced up with Victor offline on this. I will need to add the OutputFieldMappings field into the indexer.

"url": "https://docs.microsoft.com/azure/search/cognitive-search-skill-ocr"
},
"description": "The skill extracts text from image files."
"description": "A skill extracts text from image files."
Copy link
Contributor

Choose a reason for hiding this comment

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

A skill that extracts text from image files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

"url": "https://docs.microsoft.com/azure/search/cognitive-search-skill-image-analysis"
},
"description": "The skill analyzes image files. It extracts a rich set of visual features based on the image content."
"description": "A skill analyzes image files. It extracts a rich set of visual features based on the image content."
Copy link
Contributor

Choose a reason for hiding this comment

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

A skill that analyzes image files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

"url": "https://docs.microsoft.com/azure/search/cognitive-search-skill-language-detection"
},
"description": "The skill detects the language of input text and reports a single language code for every document submitted on the request. The language code is paired with a score indicating the strength of the analysis."
"description": "A skill detects the language of input text and reports a single language code for every document submitted on the request. The language code is paired with a score indicating the confidence of the analysis."
Copy link
Contributor

Choose a reason for hiding this comment

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

"that"

Why is it "every document submitted on the request"? Doesn't this skill take a single string as input?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied it from this link: https://docs.microsoft.com/azure/search/cognitive-search-skill-language-detection. I think we can send a batch request for language detection.

Copy link
Member

@jhendrixMSFT jhendrixMSFT left a comment

Choose a reason for hiding this comment

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

Looks like there's some feedback to be addressed.

@jhendrixMSFT
Copy link
Member

@vl8163264128 ping

@vl8163264128
Copy link
Contributor

#sign-off

},
"skillsetName": {
"type": "string",
"description": "The name of the cognitive skillset executing with this indexer."
Copy link
Member

Choose a reason for hiding this comment

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

@jeji1101 It would be helpful to mention here that you can leave this null if you want an indexer without an associated skillset.

"items": {
"$ref": "#/definitions/FieldMapping"
},
"description": "Output field mappings are applied after enrichment and immediately before indexing.",
Copy link
Member

Choose a reason for hiding this comment

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

@jeji1101 We don't really define the term "enrichment" here, so maybe it would help to have more context. How about:

"Applicable only to indexers with an associated skillset. Output field mappings are applied after documents are enriched by the skillset, immediately before indexing."

"$ref": "#/definitions/InputFieldMappingEntry"
},
"description": "The inputs of the skill."
"description": "Inputs of the skills could be a column in the source data set, or the output of an upstream skill."
Copy link
Member

Choose a reason for hiding this comment

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

@jeji1101 Terminology consistency: Please call it "data source" instead of "source data set".

"targetName": {
"type": "string",
"description": "The target name of the output."
"description": "The target name of the output. It is optional and default to name."
Copy link
Member

Choose a reason for hiding this comment

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

@jeji1101 default to name -> defaults to name

"required": [
"name"
],
"externalDocs": {
Copy link
Member

Choose a reason for hiding this comment

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

@jeji1101 This seems like the wrong doc link.

"url": "https://docs.microsoft.com/azure/search/cognitive-search-skill-sentiment"
},
"description": "Text analytics sentiment analysis."
"description": "Text analytics positive-negative sentiment analysis, scored as a floating point value in a range of zero to 1."
Copy link
Member

Choose a reason for hiding this comment

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

@jeji1101 "zero to one" or "0 to 1"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants