Begin allowing Site-specific data

thoughtbot / carnival

An unobtrusive, developer-friendly way to add comments

MIT License

499 stars 30 forks source link

Begin allowing Site-specific data #142

Closed pbrisbin closed 9 years ago

pbrisbin commented 9 years ago

Changes:

Create Site model
Migrations to create dev/staging/production blog records
Add /sites/#SiteId prefix to most routes
Render FeedR and EmbedR for a specific Site
EmbedR uses its SiteId in comments API calls

Deployment:

Requires migrate-deploy (create-site, insert-thoughtbot-sites)
Comments will temporarily not work, only until I modify CARNIVAL_HOST on staging/production to include the /sites/#SiteId prefix
Zapier hook will need to be changed to include /sites/#SiteId prefix

jferris commented 9 years ago

This looks like a good approach to me.

Is it difficult to put a rewrite in place for old URLs to make the transition without downtime?

We may need to disable the Zapier hook or something to prevent it from spamming our timeline.

pbrisbin commented 9 years ago

Is it difficult to put a rewrite in place for old URLs to make the transition without downtime?

It's not too difficult, but we're talking a few seconds of downtime at most, so I didn't think it was worth it. Do you disagree?

We may need to disable the Zapier hook or something to prevent it from spamming our timeline

What do you think would be spammed? I thought it just wouldn't work until updated. Are you saying that a feed URL change makes it see all existing entries as new?

jferris commented 9 years ago

What do you think would be spammed? I thought it just wouldn't work until updated. Are you saying that a feed URL change makes it see all existing entries as new?

Yeah, I'm not sure. I've had issues like this before when changing URLs. I think it's when the feed entry URLs change, but I'm not sure what it will do if the whole feed seems to change at once.

jferris commented 9 years ago

I guess the best thing to do here is probably to just check Buffer after deploying to make sure we didn't queue up hundreds of Tweets or anything.

pbrisbin commented 9 years ago

I'm pretty confident this feed is only used for our Slack notifications. We don't Tweet anything based on comments on the blog. I'll try to confirm this.

pbrisbin commented 9 years ago

I can't find any zapier integrations...

pbrisbin commented 9 years ago

So, there's nothing in Zapier based on this feed. The Slack integration is here. It looks like updating means removing and re-adding.

My assumption is that when you add an RSS-based integration, any existing entries will be ignored and only new entries will trigger the hook. Considering the worst-case result is a bunch of messages in #general, I think I'm OK moving forward with that assumption.

Are you OK with brief downtime during the deploy, or should I implement the temporary redirect before merging?

jferris commented 9 years ago

I think this is good to merge. Thanks for following up on my RSS paranoia. On Mar 3, 2015 3:14 PM, "pat brisbin" notifications@github.com wrote:

So, there's nothing in Zapier based on this feed. The Slack integration is here https://thoughtbot.slack.com/services/2417784262. It looks like updating means removing and re-adding.

My assumption is that when you add an RSS-based integration, any existing entries will be ignored and only new entries will trigger the hook. Considering the worst-case result is a bunch of messages in #general, I think I'm OK moving forward with that assumption.

Are you OK with brief downtime during the deploy, or should I implement the temporary redirect before merging?

— Reply to this email directly or view it on GitHub https://github.com/thoughtbot/carnival/pull/142#issuecomment-77025030.