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

Add support for Insight/Webtest 2022-06-15 #3911

Merged
merged 5 commits into from
Apr 17, 2024
Merged

Conversation

super-harsh
Copy link
Collaborator

@super-harsh super-harsh commented Apr 5, 2024

What this PR does / why we need it:

Closes #3294
Add support for stable Insight preview resources.

Special notes for your reviewer:

How does this PR make you feel:
gif

If applicable:

  • this PR contains documentation
  • this PR contains tests
  • this PR contains YAML Samples

@super-harsh super-harsh self-assigned this Apr 7, 2024
@super-harsh super-harsh added this to the v2.7.0 milestone Apr 7, 2024
@super-harsh super-harsh added the new-resouce-version New version of a resource ASO already supports label Apr 9, 2024
v2/samples/insights/v1api/v1api20220615_webtest.yaml Outdated Show resolved Hide resolved
@@ -2,93 +2,108 @@
github.com/Azure/azure-service-operator/v2/api/insights/v1api20180501preview/storage
├── APIVersion: Enum (1 value)
│ └── "2018-05-01-preview"
└── Webtest: Resource
Copy link
Member

Choose a reason for hiding this comment

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

minor: @theunrepentantgeek, it would be nice if the format for this file didn't change so drastically when additions were made.

Maybe pure whitespace/indentation is enough and we can drop the |? Or maybe we could just render | anyway even if there's nothing below so that when something is added the change doesn't edit all the above lines too?

LMK if you think worth filing an issue over it.

│ │ ├── Description: *string
│ │ ├── Enabled: *bool
│ │ ├── Frequency: *int
│ │ ├── Kind: *Enum (3 values)
Copy link
Member

Choose a reason for hiding this comment

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

The old API had a basic entry here... not sure what we need to do with that (if anything), but it won't roundtrip forward/backwards.

Copy link
Member

Choose a reason for hiding this comment

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

Related to #3864

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think PropertyConversion functions would handle that by adding the extra property in PropertyBag?

Not sure, calling @theunrepentantgeek here as he knows the conversions better.

namespace: default
spec:
tags:
"hidden-link:/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/aso-sample-rg/providers/microsoft.insights/components/sampleappinsights": Resource
Copy link
Member

Choose a reason for hiding this comment

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

minor: Do we know that this is still required for the new webtest type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, I'll double check and post an update.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Double-checked. Its still needed. Getting a 400 Bad request with message below:
"A single 'hidden-link' tag pointing to an existing AI component is required. Found none."

@super-harsh super-harsh added this pull request to the merge queue Apr 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 16, 2024
@super-harsh super-harsh added this pull request to the merge queue Apr 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 16, 2024
@super-harsh super-harsh added this pull request to the merge queue Apr 17, 2024
Merged via the queue into main with commit 8757da6 Apr 17, 2024
9 checks passed
@super-harsh super-harsh deleted the feature/insight branch April 17, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-resouce-version New version of a resource ASO already supports
Projects
Development

Successfully merging this pull request may close these issues.

Import stable versions of preview resources
2 participants