Re-apply Portal Dashboard: Treat Part's Metadata as untyped JSON #3566

mikhailshilkov · 2024-09-09T13:45:09Z

Re-applies #3123 after it's been reverted in #3500

The only functional change compared to the original PR is a rename of DashboardPartsResponsePosition to DashboardPartsPositionResponse. Upstream have probably shuffled type definitions in some way that makes us choose a different name.

As discussed after the initial review, I've also added two things to help avoid confusion in the future:

Asserts that the types that we rely on during overrides are actually present in the original schema
Text comments in the examples about the importance of non-markdown parts in tests

Fix #3562
Fix #1412
Fix #747
Fix https://github.com/pulumi/customer-support/issues/1737 (I imported the created dashboard and received the settings of dashboard parts. No automated test for that.)

github-actions · 2024-09-09T13:49:16Z

Does the PR have any schema changes?

Found 10 breaking changes:

Types

🟡 "azure-native:portal:DashboardParts": properties: "metadata" type changed from "#/types/azure-native:portal:MarkdownPartMetadata" to "#/types/azure-native:portal:DashboardPartMetadata"
🟡 "azure-native:portal:DashboardPartsResponse": properties: "metadata" type changed from "#/types/azure-native:portal:MarkdownPartMetadataResponse" to "#/types/azure-native:portal:DashboardPartMetadataResponse"
🔴 "azure-native:portal:MarkdownPartMetadata" missing
🔴 "azure-native:portal:MarkdownPartMetadataResponse" missing
🔴 "azure-native:portal:MarkdownPartMetadataSettings" missing
🔴 "azure-native:portal:MarkdownPartMetadataSettingsContent" missing
🔴 "azure-native:portal:MarkdownPartMetadataSettingsContentResponse" missing
🔴 "azure-native:portal:MarkdownPartMetadataSettingsContentSettings" missing
🔴 "azure-native:portal:MarkdownPartMetadataSettingsContentSettingsResponse" missing
🔴 "azure-native:portal:MarkdownPartMetadataSettingsResponse" missing
No new resources/functions.

codecov · 2024-09-09T13:57:30Z

Codecov Report

Attention: Patch coverage is 98.38710% with 2 lines in your changes missing coverage. Please review.

Project coverage is 58.39%. Comparing base (392852d) to head (7843308).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
provider/pkg/gen/schema.go	75.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3566      +/-   ##
==========================================
+ Coverage   57.77%   58.39%   +0.62%     
==========================================
  Files          66       67       +1     
  Lines        8294     8418     +124     
==========================================
+ Hits         4792     4916     +124     
+ Misses       3055     3054       -1     
- Partials      447      448       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

danielrbradley

To discuss further:

How do we make this less fragile if upstream changes again in the future?
How do we know if the dashboard is actually usable and not just deployable (as these seem to be different)?
Is this in scope to fully manage the implementation of the types as custom code or should we just ensure any value can be passed in.

mikhailshilkov · 2024-09-16T10:16:23Z

I think our long-term answer to the questions above is in #3571

Therefore, I removed the extra checks from this PR and brought it back to as pure revert as possible to minimize the probability of breaking something and the diff versus #3571

@mjeffryes @thomas11 please take a look

mjeffryes

Given the commit history, I found it difficult to verify this as a simple revert + rename, but approving the merge on that basis to get back to a non-broken state for the customer.

Comments are notes of things to make sure we improve with #3571 or whatever PR we land for a long term solution.

mjeffryes · 2024-09-16T17:20:54Z

provider/pkg/resources/customresources/customresources.go

+// SchemaTypeOverrides returns the map of custom resource schema overrides per resource token.
+func SchemaTypeOverrides() map[string]schema.ComplexTypeSpec {


Why is it SchemaTypeOverrides here, but TypeOverrides on the struct?

mjeffryes · 2024-09-16T17:21:53Z

provider/pkg/gen/schema.go

@@ -550,12 +550,24 @@ func genMixins(pkg *pschema.PackageSpec, metadata *resources.AzureAPIMetadata) e
 		}
 		pkg.Types[tok] = t
 	}
+	for tok, t := range customresources.SchemaTypeOverrides() {


This should probably be checking that the type being overridden exists?

mjeffryes · 2024-09-16T17:22:06Z

provider/pkg/gen/schema.go

+		}
+		metadata.Types[tok] = r
+	}
+	for tok, r := range customresources.MetaTypeOverrides() {


This should probably be checking that the type being overridden exists?

mjeffryes · 2024-09-16T17:23:23Z

provider/pkg/resources/customresources/customresources.go

@@ -38,6 +38,11 @@ type CustomResource struct {
 	// Resource metadata. Defines the parameters and properties that are used for diff calculation and validation.
 	// Optional, by default the metadata is assumed to be derived from Azure Open API specs.
 	Meta *AzureAPIResource
+	// MetaTypes are net-new auxiliary metadata types defined for this resource. Optional.


AZ-Native n00b question probably, but what are "auxiliary metadata types"?

mjeffryes · 2024-09-16T17:28:43Z

provider/pkg/gen/schema.go

+	}
+	for tok, r := range customresources.MetaTypeOverrides() {
+		metadata.Types[tok] = r
+	}


Nit. It's strange to me that these overrides and mixins are splatted into the package here with so little context. Why are do we do overrides after mixins? Why are are the functions above and the workspaceSqlAadAdmin description added in a totally different way?

mjeffryes · 2024-09-16T17:30:01Z

provider/pkg/resources/customresources/customresources.go

+	types := map[string]schema.ComplexTypeSpec{}
+	for _, r := range featureLookup {
+		for n, v := range r.TypeOverrides {
+			types[n] = v


no check for collisions between CustomResource instances?

pulumi-bot · 2024-09-18T12:57:32Z

This PR has been shipped in release v2.62.0.

mikhailshilkov changed the title ~~Mikhailshilkov/portal dashboard 2~~ Re-apply Portal Dashboard: Treat MarkdownPartMetadata as untyped JSON Sep 9, 2024

mikhailshilkov changed the title ~~Re-apply Portal Dashboard: Treat MarkdownPartMetadata as untyped JSON~~ Re-apply Portal Dashboard: Treat Part's Metadata and Inputs as untyped JSON Sep 9, 2024

mikhailshilkov changed the title ~~Re-apply Portal Dashboard: Treat Part's Metadata and Inputs as untyped JSON~~ Re-apply Portal Dashboard: Treat Part's Metadata as untyped JSON Sep 9, 2024

mikhailshilkov force-pushed the mikhailshilkov/portal-dashboard-2 branch from 3161932 to 0e45e16 Compare September 9, 2024 14:51

mikhailshilkov marked this pull request as ready for review September 9, 2024 15:56

mikhailshilkov requested review from danielrbradley and thomas11 September 9, 2024 15:56

danielrbradley reviewed Sep 9, 2024

View reviewed changes

mikhailshilkov force-pushed the mikhailshilkov/portal-dashboard-2 branch 5 times, most recently from a6b42c6 to e1f6efd Compare September 16, 2024 10:07

mikhailshilkov added 6 commits September 16, 2024 12:09

Custom implementation of portal.Dashboard

4246cd6

Schema

ae58d65

Node.js

b7bff44

Python

3440080

.NET

3715d51

Example

7843308

mikhailshilkov force-pushed the mikhailshilkov/portal-dashboard-2 branch from e1f6efd to 7843308 Compare September 16, 2024 10:09

mjeffryes approved these changes Sep 16, 2024

View reviewed changes

mikhailshilkov merged commit fb1064e into master Sep 17, 2024
23 checks passed

mikhailshilkov deleted the mikhailshilkov/portal-dashboard-2 branch September 17, 2024 08:42

This was referenced Sep 18, 2024

Dashboards are unusable again #3562

Closed

Azure native package creates invalid dashboard and the current configuration does not allow us to pass valid metadata property. #1412

Closed

Unsupported Dahsboard types in azure-native/portal/Dashboard #747

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-apply Portal Dashboard: Treat Part's Metadata as untyped JSON #3566

Re-apply Portal Dashboard: Treat Part's Metadata as untyped JSON #3566

mikhailshilkov commented Sep 9, 2024 •

edited

Loading

github-actions bot commented Sep 9, 2024 •

edited

Loading

codecov bot commented Sep 9, 2024 •

edited

Loading

danielrbradley left a comment

mikhailshilkov commented Sep 16, 2024

mjeffryes left a comment

mjeffryes Sep 16, 2024

mjeffryes Sep 16, 2024

mjeffryes Sep 16, 2024

mjeffryes Sep 16, 2024

mjeffryes Sep 16, 2024

mjeffryes Sep 16, 2024

pulumi-bot commented Sep 18, 2024

		// SchemaTypeOverrides returns the map of custom resource schema overrides per resource token.
		func SchemaTypeOverrides() map[string]schema.ComplexTypeSpec {

Re-apply Portal Dashboard: Treat Part's Metadata as untyped JSON #3566

Re-apply Portal Dashboard: Treat Part's Metadata as untyped JSON #3566

Conversation

mikhailshilkov commented Sep 9, 2024 • edited Loading

github-actions bot commented Sep 9, 2024 • edited Loading

Does the PR have any schema changes?

Types

codecov bot commented Sep 9, 2024 • edited Loading

Codecov Report

danielrbradley left a comment

Choose a reason for hiding this comment

mikhailshilkov commented Sep 16, 2024

mjeffryes left a comment

Choose a reason for hiding this comment

mjeffryes Sep 16, 2024

Choose a reason for hiding this comment

mjeffryes Sep 16, 2024

Choose a reason for hiding this comment

mjeffryes Sep 16, 2024

Choose a reason for hiding this comment

mjeffryes Sep 16, 2024

Choose a reason for hiding this comment

mjeffryes Sep 16, 2024

Choose a reason for hiding this comment

mjeffryes Sep 16, 2024

Choose a reason for hiding this comment

pulumi-bot commented Sep 18, 2024

mikhailshilkov commented Sep 9, 2024 •

edited

Loading

github-actions bot commented Sep 9, 2024 •

edited

Loading

codecov bot commented Sep 9, 2024 •

edited

Loading