-
Notifications
You must be signed in to change notification settings - Fork 187
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
WCA Live Integration Pre Work #1: Job and Websockets #10684
base: main
Are you sure you want to change the base?
Conversation
Just curious, what do you mean by "WCA Live integration"? Are there any documents describing this available? |
We're slowly, gradually absorbing WCA Live into the main website. This is a month(s)-long project, and as such we don't have detailed documentation available. But the gist of it is: Get rid of the separate URL and "Sync Now" |
I'd also be interested or hear more about this. One of the primary motivators for WCIF was to make it possible for people to build a pieces of competition infrastructure without having to build everything, and without having to yeet code at the WST to host (leaving the WST responsible for more than it can/wants to handle). Concretely, (and meaning no disrespect to WCA Live here): after this integration is complete, would it be possible for a community member to build and use an alternative to WCA Live? |
Before we dive any deeper, I'd like to clarify that we're not getting rid of WCIF at all. The existing endpoints for custom pieces of competition infrastructure will continue to exist and be maintained. For WCA Live specifically, it was started as a custom piece of software, but quickly became so popular that (outside of a few. select regions) it is the de-facto standard for running WCA competitions. And it has already been "yeeted at the WST", so we are already responsible for maintaining it. Right now, that responsibility rests solely on Jonatan's shoulders. We want to change this and reduce the truck factor, so we have to come up with an integration that is maintainable for a broader audience. There were lengthy discussions last year involving the Board, paid contributors, and Jonatan, and we figured out together that this "absorption" into the main website would be the best way forward. Here are the core arguments at a glance:
Furthermore, I can also already hint at the fact that this "merge refactor" will have another cool side effect: We will open up the API to post a live result, so that other things like TimeBase can submit their results and have them appear directly, just as manual results entry would. Lastly, results submission will be greatly simplified by this and it will allow us to easily implement some quality of life changes and data integrity checks that WRT has been yearning for for years. |
Thanks Gregor for detailed explanation, I agree with you in almost all aspects. One thing I'd argue about is MUI, I think it's super cool for Live and much better than SemUI for admin things, like data entry. But at the same time I understand your point about consistency.
Just wanted to note that WCA Live API already exists and is doing great ;) |
That largely makes sense! 100% agree that if the WST is effectively already maintaining WCA Live, then it'll be easier to maintain if it lives inside of the rails monolith. I'm not sure you answered my last question, though:
|
I would say that it would be even easier, as the community could just use the same APIs that we are building for this new WCA Live integration. We have talked about the possibility of working with cubing china to see if they can set up their live results system that way (honestly I don't even know if they are even using WCIF or not just doing everything by themselves and then dumping the JSON into the WCA in the end!) |
self.queue_adapter = :shoryuken if Rails.env.production? && !EnvConfig.WCA_LIVE_SITE? | ||
queue_as EnvConfig.LIVE_QUEUE if Rails.env.production? && !EnvConfig.WCA_LIVE_SITE? |
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.
Can you move the if
check into a globally accessible WcaLive.enabled?
variable like we did with paypal?
# identified_by :current_user | ||
# | ||
def connect | ||
reject_unauthorized_connection if EnvConfig.WCA_LIVE_SITE? |
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.
In line with my other comment: reject unless WcaLive.enabled?
queue_as EnvConfig.LIVE_QUEUE if Rails.env.production? && !EnvConfig.WCA_LIVE_SITE? | ||
|
||
def perform(params) | ||
ActionCable.server.broadcast("results_#{params[:round_id]}", |
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.
Also, please share this broadcast key (perhaps in the same module as the "WCA Live enabled?" feature above)
|
||
class LiveResultsChannel < ApplicationCable::Channel | ||
def subscribed | ||
stream_from "results_#{params[:round_id]}" |
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.
Shared constant yay
I will do the database changes in a follow up PR so currently I am just directly broadcasting from the job (instead of in an
after_save
hook).I have a test page, should I commit that too?