New Tool: Data API Differ #3462
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit introduces one struct per Type of Change - allowing us to pick out later on which Changes are relevant for which Command.
…ls for now This is necessary until the Data API SDK is updated - but can be reconciled later
This'll need to be configured both in any Tests and the Main function, but allows this to be switched out as needed
This allows detecting the changes between two sets of Resource Manager Services, but in the future can be extended to also support Microsoft Graph when the SDK exposes it. This allows detecting of changes in: * Services * API Versions * API Resources * Constants * Models * Operations (and Operation Options) * Resource IDs (and Resource ID Segments) This currently _doesn't_ support tracking changes to Terraform Resources, but that seems like a good logical extension point for the future.
…list of Static Identifiers This allows us to compute the list of Resource ID Segments which are new/updated so we can output a static list
This makes use of a "View" concept similar to Terraform Core, the intention here being to allow us the ability to output this as JSON in the future for interoperability with other tooling (such as the `upgrade-go-azure-sdk` tool in `hashicorp/go-azure-sdk`). The Changes and Breaking Changes UIs require further work to clean these up - this is very much an MVP showing all of the available data but in the future I suspect we'll change this to be grouped by Data Type (e.g. Resource Manager/MS Graph) and Service and have the nested information expandable, or something. But this is fine for an MVP. Noteably this will output a LOT of data in the short-term - showing ALL changes that have happened to a Service/API Version/API Resource/etc when it's added - but shows only the top-level removal (e.g. the Removal of the Service, and not each API version) when the Service and all nested items are removed.
These might seem excessive, but since we're relying on this output this is worth testing.
This helps determine when the Data API has launched, and on which port.
In the event that `data-api-binary-path` is unspecified, we now look for this on the PATH which is helpful when running in automation.
This allows outputting the rendered output to a file, which makes it possible to use the result in automation.
…n output the result on Pull Requests This triggers on changes to the API Definitions (to handle these changing), changes to the Data API (so we can see the result) and on changes to `importer-rest-api-specs`. This currently doesn't run for changes to `importer-msgraph-metadata` but can be added when the Data API SDK is updated in the near future.
This is used by the Data API Differ automation
This means that we have access to the `main` branch to be able to clone it
This was referenced Dec 14, 2023
mbfrahry
approved these changes
Dec 14, 2023
|
Yeah I'm anticipating that we'll change the output UI after having learned more about what we're after - the information being output is useful but a little unwieldily at the moment - but we can change that more easily once we've got some more examples. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a new tool
data-api-differwhich detects and then outputs the changes between two different sets of API Definitions.The intention here is to provide a replacement for the older
extract-resource-id-segmentstool - but also to output a more granular diff of what’s happened so that anyone looking at a PR which updates the API Definitions has a clear understanding of what’s changing in that Pull Request.This tool detects three types of changes:
A shortened example of the output for each of these commands can be seen in the README - but the output from an earlier build can be found in this Gist for review.
This tool detects each Type of Change as unique struct - meaning that it’s possible to quickly add more granular reports as needed - and I suspect we’ll want to extend this to support diffing two different Versions of two APIs (e.g.
2020-01-01and2023-06-01) in the future.This PR also adds the associated automation, running this tool against Pull Requests to be able to detect changes between
mainand a given Pull Request.Fixes #3315
Notes / Limitations
Model has a new Parent Type) has no backing documentation. We should look to add documentation to./docswhich explains what each of the types of changes are, and the consequence of this/if these are resolved.hashicorp/terraform-provider-azurerm(orhashicorp/terraform-provider-azuread) but that seems like a logical extension to be able to determine when a breaking change is fine (e.g. removing an unused old Service/API Version) and when it’s problematic (e.g. removing a Service/API Version which is currently vendored into the Provider[s]).data-api-reporter- however given it could be worth extending this in the future (e.g. so that it integrates with theupgrade-go-azure-sdktool) - I suspect it’s probably worth leaving this asdifferfor the moment?