Skip to content
This repository has been archived by the owner on Oct 11, 2023. It is now read-only.

Consider x-ms-client-name when prechecking for duplicate schemas #280

Merged
merged 1 commit into from
May 12, 2020

Conversation

daviwil
Copy link
Contributor

@daviwil daviwil commented May 5, 2020

This PR fixes an issue that @pakrym identified in a comment for PR #274 which reported that the Cognitive Search spec was reporting a duplicated schema even after a transform was being used to add x-ms-client-name. The change adds a check for x-ms-client-name when grouping schemas by name for deduplication purposes.

I also added an expanded message at @pakrym's request for when a PreCheck/DuplicateSchema error is raised:

ERROR (PreCheck/DuplicateSchema): Duplicate Schema named SearchError -- properties.details.$ref: undefined => "#/components/schemas/schemas:481",x-ms-client-name: "IndexSearchError" => <none> ; This error can be *temporarily* avoided by using the 'modelerfour.lenient-model-deduplication' setting.  NOTE: This setting will be removed in a future version of @autorest/modelerfour; schemas should be updated to fix this issue sooner than that.

The expanded message informs the user about the modelerfour.lenient-model-deduplication setting so that partner teams can turn it on for their specs temporarily before we remove it in a later version.

@daviwil daviwil requested a review from fearthecowboy May 5, 2020 23:35
@azure-pipelines
Copy link

You may test this build by running autorest --reset and then either:


add --use: to the command line:

autorest --use:http://tinyurl.com/ycu7z9rp ...


or use the following in your autorest configuration:

use-extension:
  "@autorest/modelerfour": "http://tinyurl.com/ycu7z9rp" 

If this build is good for you, give this comment a thumbs up. (👍)

And you should run autorest --reset again once you're finished testing to remove it.

@azure-pipelines
Copy link

You may test this build by running autorest --reset and then either:


add --use: to the command line:

autorest --use:http://tinyurl.com/yb8dtssb ...


or use the following in your autorest configuration:

use-extension:
  "@autorest/modelerfour": "http://tinyurl.com/yb8dtssb" 

If this build is good for you, give this comment a thumbs up. (👍)

And you should run autorest --reset again once you're finished testing to remove it.

@pakrym
Copy link

pakrym commented May 12, 2020

The original version worked.

@daviwil
Copy link
Contributor Author

daviwil commented May 12, 2020

Got confirmation from @pakrym that the latest push also works as expected. Merging it!

@daviwil daviwil merged commit 7915cce into Azure:master May 12, 2020
@daviwil daviwil deleted the dedupe-with-client-name branch May 12, 2020 23:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants