Skip to content

fix: edge case where pubs with sibling edges of different domains would 500#2948

Merged
gabestein merged 5 commits intomasterfrom
fix/sibling-edges
Dec 14, 2023
Merged

fix: edge case where pubs with sibling edges of different domains would 500#2948
gabestein merged 5 commits intomasterfrom
fix/sibling-edges

Conversation

@tefkah
Copy link
Copy Markdown
Member

@tefkah tefkah commented Dec 14, 2023

  • fix: include community in pub edge includes in for sibling edges to prevent weird connection 500 edge case
  • fix: revert base hostname for pubdoc to be req.hostname as that has been proven to work

Issue(s) Resolved

Test Plan

Screenshots (if applicable)

Optional

Notes/Context/Gotchas

Supporting Docs

@tefkah tefkah requested a review from gabestein December 14, 2023 20:10
gabestein

This comment was marked as outdated.

Copy link
Copy Markdown
Member

@gabestein gabestein left a comment

Choose a reason for hiding this comment

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

Found a small issue here that prevents merging, unfortunately. On custom domain pages with any connections at all, it will set the surrogate-key to the community subdomain rather than the hostname, which will prevent purging of any custom domain pages with surrogate keys.

E.g., on https://duqduqdomaintest.underlay.org/pub/zkme9ef3/release/9, surrogate-key is gabestein.duqduq.org rather than duqduqdomaintest.underlay.org

@pubpubBot pubpubBot requested a deployment to pubpub-pipel-fix-siblin-p6czz1 December 14, 2023 21:15 Abandoned
@tefkah
Copy link
Copy Markdown
Member Author

tefkah commented Dec 14, 2023

Hmm i think this should work, I reverted this problem from the previous commit. AFAIK the one on duqduq is the previous one right?
This PR has the same behavior in that regard as the one on prod rn

@gabestein
Copy link
Copy Markdown
Member

Ah, yes, you're right, this isn't on duqduq yet. Sorry, I'll revert my commit.

Comment on lines -218 to +221
[getCorrectHostname(subdomain, domain)] as string[],
[req.hostname] as string[],
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i mean this change here: above is how it is on duqduq, below is how it is on prod

@pubpubBot pubpubBot temporarily deployed to pubpub-pipel-fix-siblin-p6czz1 December 14, 2023 21:17 Inactive
@gabestein gabestein self-requested a review December 14, 2023 21:18
@gabestein gabestein merged commit c2c9d9b into master Dec 14, 2023
@gabestein gabestein deleted the fix/sibling-edges branch December 14, 2023 21:18
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.

3 participants