Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Conversation

@zrhoffman
Copy link
Member

@zrhoffman zrhoffman commented Feb 16, 2023

This PR adds the Traffic Portal v2 CDN detail page.


Which Traffic Control components are affected by this PR?

  • Traffic Portal v2

What is the best way to verify this PR?

Run the unit tests and the end-to-end tests

PR submission checklist

@zrhoffman zrhoffman added new feature A new feature, capability or behavior low impact affects only a small portion of a CDN, and cannot itself break one experimental a feature/component not directly supported by ATC Traffic Portal v2 Related to the experimental Traffic Portal version 2 labels Feb 16, 2023
@ocket8888 ocket8888 self-assigned this Feb 16, 2023
Copy link
Contributor

@ocket8888 ocket8888 left a comment

Choose a reason for hiding this comment

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

Just like in TPv1, this form allows editing the ALL CDN, which - to my chagrin - ought not to be done because that's a special meta object. Similarly, TPv1 allows editing the root tenant (it'll never succeed because the API won't allow it, but the portal itself doesn't stop you from trying), however, in TPv2 you'll notice that all of the fields of the root tenant are read-only.

That's not, strictly speaking, the same situation, because the ALL CDN actually can be successfully updated i.e. the API allows it. Though that's honestly quite dangerous, so I suggest we not allow it through the UI.

@zrhoffman
Copy link
Member Author

Rebased to get #7363

@zrhoffman
Copy link
Member Author

Just like in TPv1, this form allows editing the ALL CDN, which - to my chagrin - ought not to be done because that's a special meta object. Similarly, TPv1 allows editing the root tenant (it'll never succeed because the API won't allow it, but the portal itself doesn't stop you from trying), however, in TPv2 you'll notice that all of the fields of the root tenant are read-only.

That's not, strictly speaking, the same situation, because the ALL CDN actually can be successfully updated i.e. the API allows it. Though that's honestly quite dangerous, so I suggest we not allow it through the UI.

That's a little out-of-scope for creating the CDN details page IMO, let's create a new GH Issue for it.

@zrhoffman zrhoffman requested a review from ocket8888 February 28, 2023 16:12
@zrhoffman
Copy link
Member Author

Rebased to fix conflicts

@zrhoffman zrhoffman requested a review from ocket8888 March 27, 2023 14:04
@zrhoffman zrhoffman requested a review from ocket8888 March 28, 2023 15:05
Copy link
Contributor

@ocket8888 ocket8888 left a comment

Choose a reason for hiding this comment

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

The form needs error hints but that's systemic to TPv2 so it's unfair to require them in this PR.

@ocket8888 ocket8888 merged commit 7741530 into apache:master Mar 29, 2023
@zrhoffman zrhoffman deleted the tpv2-cdns-detail branch May 12, 2023 20:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

experimental a feature/component not directly supported by ATC low impact affects only a small portion of a CDN, and cannot itself break one new feature A new feature, capability or behavior Traffic Portal v2 Related to the experimental Traffic Portal version 2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants