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

Upgrade to Panda v7 - support key rotation #1747

Merged
merged 6 commits into from
Jan 22, 2025
Merged

Upgrade to Panda v7 - support key rotation #1747

merged 6 commits into from
Jan 22, 2025

Conversation

davidfurey
Copy link
Member

This upgrades Panda from v4 to v7, allowing us to use key rotation.

Follows the guidance from guardian/pan-domain-authentication#160 and based on guardian/atom-workshop#361

Testing

@davidfurey davidfurey requested a review from a team as a code owner January 8, 2025 16:49
@davidfurey
Copy link
Member Author

I've resolved two compiler warnings in a way which might change some behaviour that I'm unfamiliar with

@jonathonherbert - can you review 4a41ba5 ?

@Divs-B - can you review ba50498

case Some(CardType.Chef) =>
EditionsChef(
supportingCard.id,
supportingCard.frontPublicationDate,
supportingCard.meta.map(_.toChefMetadata)
)
case _ =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @davidfurey, thanks for asking,
If I remember, we do need case matching for Recipe and Chef only, we don't want this to work for Feast-Collection(FC), because we don't have yet supporting feature of FC (present inside another FC) to drag into some another FC.
Can we have undefined or something similar if we really need to do case _?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've discussed with Divya and decided to remove this change. Divya might look at resolving this warning in a later PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for discussion @davidfurey to discuss on ways to solve this.
I will make change in later PR and attach here as reference.

@davidfurey davidfurey force-pushed the bump-panda branch 2 times, most recently from 994dc0c to 9f69e01 Compare January 17, 2025 09:27
@davidfurey
Copy link
Member Author

Tested on CODE - auth with an existing panda cookie works, and logging in from incognito window also works

@jonathonherbert
Copy link
Contributor

@jonathonherbert - can you review 4a41ba5 ?

This is fine, nothing but recipes should be in FeastCollection meta, and collect is the idiomatic way to do this 👍

This upgrades Panda from v4 to v7, allowing us to use key rotation as introduced with guardian/pan-domain-authentication#150.
Implicit definition should have explicit type
Also removing unused endpoints
A pure expression does nothing in statement position
@davidfurey davidfurey merged commit 73f0842 into main Jan 22, 2025
13 checks passed
@davidfurey davidfurey deleted the bump-panda branch January 22, 2025 12:12
@prout-bot
Copy link

Seen on PROD (merged by @davidfurey 6 minutes and 23 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants