-
Notifications
You must be signed in to change notification settings - Fork 921
Mockturtle irc bot port to ruby #1637
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
Conversation
| @db = db | ||
| end | ||
|
|
||
| def watch_github |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to fix persistence so we can actually start scraping stories.
Should probably fail open and set lastSeen as a class variable. So even if we don't have working persistence we can still scrape rss.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. We should use ActiveRecord here. For all I grumble about it, the consistency of matching the site database access would be worth it.
pushcx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Off to a good start here, and I've seen the activity in the test channel. I've added some Ruby pointers and I look forward to seeing this in prod. I'll talk to you offline about that idea of setting up some pairing time.
mockturtle/README.md
Outdated
| ## License | ||
| ``` | ||
| Copyright 2025 Noah Hendrickson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to use the license that's already used for the site (3 clause BSD, if my quick glance is correct) because code will be shared between the bot and site. Do you have a strong preference that we do it differently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No strong preference, will copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're using the same license, you can probably just delete the license from this README and the whole repo is covered by the top level license.
| @db = db | ||
| end | ||
|
|
||
| def watch_github |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. We should use ActiveRecord here. For all I grumble about it, the consistency of matching the site database access would be worth it.
| require "time" | ||
|
|
||
| class Seen | ||
| class SeenStruct < Struct.new(:who, :where, :time) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these aren't modified after creation, recent versions of Ruby (which we're running) have a Data class that's immutable.
I'd also prefer to just be Seen without the hungarian notation. Ruby is very timey-wimey about types.
Gemfile
Outdated
| gem "lograge" # for JSON logging | ||
| gem "silencer" # to disable default logging in prod | ||
|
|
||
| # mockturtle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this is going to run in a separate process, let's make a mockturtle/Gemfile for it. I don't want the Rails processes to take up any more RAM than they already do.
Gemfile
Outdated
| gem "ircinch" | ||
| gem "json" | ||
| gem "nokogiri" | ||
| gem "open-uri" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this and use extras/sponge.rb to fetch anything. It prevents people from saying in chat "https://127.0.0.1:22" etc and using the bot to portscan/exploit us or others.
Gemfile
Outdated
| gem "open-uri" | ||
| gem "rss" | ||
| gem "sqlite3" | ||
| gem "time" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed, it's in the standard library and loaded automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need it in the Gemfile, we do need to require it (which we might be getting for free with the Gemfile entry?)
Methods like Time.parse() aren't loaded unless you explicitly require the time stdlib.
|
Open integration questions for myself:
|
We should be able to configure ActiveRecord readonly connections, the safest way would be to have a readonly Postgres user the bot connects as.
The bot could listen on a local port or socket, and have the app make a request in the background to the bot over localhost to trigger it? |
eccd0ea to
e42d35b
Compare
| gem "ircinch" | ||
| gem "rss" | ||
|
|
||
| # performance monitoring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # performance monitoring | |
| # bug reporting |
|
Building on @caius's comment, I think we could cheaply get a robust way for the rails app to boss the irc bot around by using Solid Queue. The rails app could write jobs to the queue for a worker process in the mockturtle process to run. This saves us writing our own code for things like serialization or supervising the worker. Solid Queue includes a puma plugin for that process to supervise a solid queue worker, and I think this is very close to what we'd want. So maybe we get something very mature very cheaply! |
|
This PR has had no activity for 21 days. Please leave a comment or commit to indicate you're still actively working on it. |
|
This PR has had no activity for 21 days. Please leave a comment or commit to indicate you're still actively working on it. |
|
This PR was closed because it was open for 28 days with no activity. Please reopen when you'd like to continue work on this. |
Basic port of the current mockturtle nodeJS codebase to a ruby port under the main lobsters repo.
Implements the same dot commands mockturtle currently does as well as the rss watching notifications we already have for new site stories and repo commits.
Still needs: