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

Fetch articles via gollum #53

Closed
wants to merge 9 commits into from
Closed

Conversation

semmons99
Copy link

@sandal: Here the first shot at fetching articles via Gollum; works pretty nice. Let me know if you have any questions or need any changes made.

TODO:

  • Automatically create git repo for user to contain articles
  • Derive name based on volume and article numbers

@jordanbyron
Copy link
Contributor

@semmons99 cool stuff. Here are a few things I've noticed so far:

  1. When edits are made directly to the article file via git, we need to make sure the cache is invalidated. That can either be done by adding the datetime modified to the article's cache key or using some sort of git hook to manually mark the cache as invalidated. Personally I think the last modified datetime makes the most sense here.
  2. Rather than specifying a path for each article manually, why don't we use a standard naming convention and path to look up articles automatically. I'm not crazy about database ids, but perhaps issue + article number: /gollum/articles/1.15.md. Talk to @sandal about that one.
  3. Which leads into my next point that I think we should have a standard path for gollum repos in the app. /gollum comes to mind, and within that folder we'd create subfolders for different resources (articles, announcements, etc). Each of those subfolders would be their own git repos. Then that's one less thing to configure when setting up the app. Plus with symlinks that folder can really be located anywhere, so we still have the same flexibility as creating a custom path in the initializer.

These are more questions / thoughts.

  • Does Gollum::Wiki.new create a git repo in the directory you pass in if it doesn't exist? If it doesn't we might want to do that automatically for new developers, or at the very least add it to the README
  • Are we planning to eventually hook gollum into the app's administrator interface, so authors can make minor edits to their articles without having to crack open their editor? I think that would be nice, but maybe not a v1.0 critical feature.

That's about all I can think of right now. Again thanks for hacking this out @semmons99 🚒

@semmons99
Copy link
Author

  • Does Gollum::Wiki.new create a git repo in the directory you pass in if it doesn't exist? If it doesn't we might want to do that automatically for new developers, or at the very least add it to the README

No, it requires that you have at least initialized a git repo

@practicingruby
Copy link
Member

Rather than specifying a path for each article manually, why don't we use a standard naming convention and path to look up articles automatically. I'm not crazy about database ids, but perhaps issue + article number: /gollum/articles/1.15.md. Talk to @sandal about that one.

I'm okay with this, but the pattern should be this: "1.15.*.md"

This will let me write names in the file if I want, like 1.15.the-article-name.md

@semmons99
Copy link
Author

@jordanbyron: I'm heading out for a week of vacation on Monday, so I'm dumping all my questions out before I leave. Here goes.

  1. When edits are made directly to the article file via git, we need to make sure the cache is invalidated. That can either be done by adding the datetime modified to the article's cache key or using some sort of git hook to manually mark the cache as invalidated. Personally I think the last modified datetime makes the most sense here.

Greg and I were talking about this today. We're going to already need an after_commit hook that moves data from the practicing-ruby repo into the subfolder gollum repo in practicing-ruby-web (probably some type of capistrano task). So we were thinking we'd just invalidate the cache at the same type via the task. Does this sound feasible to you?

Next question, any idea what's caused travis to fail? I don't see any errors in that log.

I'll post more questions/responses through the weekend as I think of them.

@semmons99
Copy link
Author

@jordanbyron, @sandal: I've been thinking about this more today, specifically keeping the articles updated and invalidating the cache. What if we setup a Post-Recieve webhook that fires when elm-city-craftworks/practicing-ruby is updated? When we recieved that POST, the gollum repo can be refreshed and using the commit information provided by Github, we could invalidate the cache. Thoughts?

@practicingruby
Copy link
Member

@semmons99: That's worth looking into, since we already need to have a web app up and running anyway. What do you think @jordanbyron ?

@jordanbyron
Copy link
Contributor

I think that could work. The tricky part will be determining which articles were updated and therefore need to be invalidated. This might be a good opportunity for us to explore adding modified times to our cache keys. That would automatically invalidate the cache for us when a view template or article file is updated. Then we can have CacheCooker hit the entire site to refresh anything that was invalidated.

But a less intense v1 might be able to leverage the commit data to determine which caches need to be invalidated and refreshed. I'm just not 100% sure how that would work.

@semmons99
Copy link
Author

How horrible would it be to have a v1.0 invalidate the entire cache?

@jordanbyron
Copy link
Contributor

@semmons99 it would be no different than our current deploy strategy, as long as that process includes running rake bake:articles. However that is not ideal, because any minor update to one article means for a period of about a minute, the site will be slow to respond if you happen to beat CacheCooker to the punch. And if @sandal is constantly making minor revisions, pushing, and checking them out on the site, then we've got a major problem on our hands.

@practicingruby
Copy link
Member

@semmons99 The point @jordanbyron raised is an important one: I regularly do tons of edits in rapid succession against the live site. With that in mind, I suppose for a v1 I'd be almost happy with a manual sync button on articles in the web app.

@jordanbyron
Copy link
Contributor

@semmons99 just wanted to give you a heads up that @sandal and I are re-thinking this feature and I've dumped our thoughts here: #57. We can discuss this in more detail today during our call ❤️

@semmons99
Copy link
Author

Sounds good!

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