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 the ceasedToBeCanonicalAt datetime to evolved URL aliasPaths #197

Merged
merged 2 commits into from
Feb 24, 2021

Conversation

JustinPinner
Copy link
Member

What does this change?

Ophan require visibility of the date at which an alias ceased to reflect the canonical path for the content. This PR restructures the thrift model by removing the previous string representation and applies a new AliasPath type.

How to test

Ensure that aliasPaths are properly represented in Concierge responses, capi scala client models, crier events, pubflow monitoring events and fastly decache events.

  • Tested on LOCAL environment (as much as possible)
  • Tested on CODE environment

How can we measure success?

The correct data appears in the correct places in the correct format? Success!

Have we considered potential risks?

We're now changing more of the existing model but the feature isn't widely adopted yet, and because use of these are opt-in we don't imaging seeing any unexpected failures.

Images

N/A

Copy link
Contributor

@frj frj left a comment

Choose a reason for hiding this comment

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

Looks good!


1: required string path

2: required CapiDateTime ceasedToBeCanonicalAt
Copy link
Contributor

Choose a reason for hiding this comment

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

CapiDateTime is interesting; is there nothing more global available?
Looks like it's existing though so happy probably consistant with what other fields do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I just took a lead from elsewhere. I think it's to provide iso8601 formatting as standard.

Copy link
Contributor

@tonytw1 tonytw1 left a comment

Choose a reason for hiding this comment

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

Looks legit.
Retires fields which were optional which is safe.

@JustinPinner JustinPinner merged commit 3d15b5a into master Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants