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.

Sign up for GitHub

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

Jump to bottom

Send only data that is needed on the dashboard #173

Closed
wants to merge 6 commits into from
Closed

Send only data that is needed on the dashboard #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a72d94d
remove weird spaces
toy Sep 2, 2020
dff0e3f
send only data that is needed on the dashboard
toy Sep 2, 2020
0b0b5ee
inline latest_events method
toy Sep 2, 2020
5f5f31e
fix tests to work with connections being a Hash instead of an Array
toy Feb 24, 2021
97a72a4
correct closing streams when stopping the server
toy Feb 24, 2021
4216c86
allow all dashboards events to pass through
toy Feb 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 35 additions & 30 deletions javascripts/dashing.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ class Dashing.Widget extends Batman.View
super

@mixin($(@node).data())
Dashing.widgets[@id] ||= []
Dashing.widgets[@id].push(@)
if @id # skip widgets without id
Dashing.widgets[@id] ||= []
Dashing.widgets[@id].push(@)

type = Batman.Filters.dashize(@view)
$(@node).addClass("widget widget-#{type} #{@id}")
Expand Down Expand Up @@ -121,34 +122,38 @@ Dashing.widgets = widgets = {}
Dashing.lastEvents = lastEvents = {}
Dashing.debugMode = false

source = new EventSource('events')
source.addEventListener 'open', (e) ->
console.log("Connection opened", e)

source.addEventListener 'error', (e)->
console.log("Connection error", e)
if (e.currentTarget.readyState == EventSource.CLOSED)
console.log("Connection closed")
setTimeout (->
window.location.reload()
), 5*60*1000

source.addEventListener 'message', (e) ->
data = JSON.parse(e.data)
if lastEvents[data.id]?.updatedAt != data.updatedAt
if Dashing.debugMode
console.log("Received data for #{data.id}", data)
lastEvents[data.id] = data
if widgets[data.id]?.length > 0
for widget in widgets[data.id]
widget.receiveData(data)

source.addEventListener 'dashboards', (e) ->
data = JSON.parse(e.data)
if Dashing.debugMode
console.log("Received data for dashboards", data)
if data.dashboard is '*' or window.location.pathname is "/#{data.dashboard}"
Dashing.fire data.event, data
Dashing.on 'run', ->
setTimeout -> # run only when all widgets are created
ids = Object.keys(Dashing.widgets)
source = new EventSource('events?ids=' + encodeURIComponent(ids.join(',')))
source.addEventListener 'open', (e) ->
console.log("Connection opened", e)

source.addEventListener 'error', (e)->
console.log("Connection error", e)
if (e.currentTarget.readyState == EventSource.CLOSED)
console.log("Connection closed")
setTimeout (->
window.location.reload()
), 5*60*1000

source.addEventListener 'message', (e) ->
data = JSON.parse(e.data)
if lastEvents[data.id]?.updatedAt != data.updatedAt
if Dashing.debugMode
console.log("Received data for #{data.id}", data)
lastEvents[data.id] = data
if widgets[data.id]?.length > 0
for widget in widgets[data.id]
widget.receiveData(data)

source.addEventListener 'dashboards', (e) ->
data = JSON.parse(e.data)
if Dashing.debugMode
console.log("Received data for dashboards", data)
if data.dashboard is '*' or window.location.pathname is "/#{data.dashboard}"
Dashing.fire data.event, data
, 0

$(document).ready ->
Dashing.run()
25 changes: 11 additions & 14 deletions lib/dashing/app.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def authenticated?(token)
set :assets_prefix, '/assets'
set :digest_assets, false
set :server, 'thin'
set :connections, []
set :connections, {}
set :history_file, 'history.yml'
set :public_folder, File.join(settings.root, 'public')
set :views, File.join(settings.root, 'dashboards')
Expand All @@ -54,7 +54,7 @@ def authenticated?(token)
settings.sprockets.append_path("assets/#{path}")
end

['widgets', File.expand_path('../../../javascripts', __FILE__)]. each do |path|
['widgets', File.expand_path('../../../javascripts', __FILE__)].each do |path|
settings.sprockets.append_path(path)
end

Expand All @@ -77,9 +77,12 @@ def authenticated?(token)
get '/events', :provides => 'text/event-stream' do
protected!
response.headers['X-Accel-Buffering'] = 'no' # Disable buffering for nginx
ids = params[:ids].to_s.split(',').to_set
stream :keep_open do |out|
settings.connections << out
out << latest_events
settings.connections[out] = ids
settings.history.each do |id, event|
out << event if ids.include?(id)
end
out.callback { settings.connections.delete(out) }
end
end
Expand Down Expand Up @@ -131,7 +134,7 @@ def authenticated?(token)

Thin::Server.class_eval do
def stop_with_connection_closing
Sinatra::Application.settings.connections.dup.each(&:close)
Sinatra::Application.settings.connections.dup.each_key(&:close)
stop_without_connection_closing
end

Expand All @@ -141,12 +144,12 @@ def stop_with_connection_closing

def send_event(id, body, target=nil)
body[:id] = id
body[:updatedAt] ||= (Time.now.to_f * 1000.0).to_i
body[:updatedAt] ||= (Time.now.to_f * 1000.0).to_i
event = format_event(body.to_json, target)
Sinatra::Application.settings.history[id] = event unless target == 'dashboards'
Sinatra::Application.settings.connections.each { |out|
Sinatra::Application.settings.connections.each { |out, ids|
begin
out << event
out << event if target == 'dashboards' || ids.include?(id)
rescue IOError => e # if the socket is closed an IOError is thrown
Sinatra::Application.settings.connections.delete(out)
end
Expand All @@ -159,12 +162,6 @@ def format_event(body, name=nil)
str << "data: #{body}\n\n"
end

def latest_events
settings.history.inject("") do |str, (_id, body)|
str << body
end
end

Copy link
Member

Choose a reason for hiding this comment

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

Just confirming, I think this isn't necessarily needed for the change, but since it's not used we are good to remove it, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is not used. To note that the way I "inlined" it changes the behaviour as before all events were sent as one blob and with the change events are sent one by one.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. Had to read the code again to see which part you were talking about.

If I understand it right, before we started the event stream and pushed all changes at once. Now we iterate the events filtering the ones we want, and pushing one by one.

I cannot think of a better way right now, and at least testing locally, I didn't note any performance issues.

def first_dashboard
files = Dir[File.join(settings.views, '*')].collect { |f| File.basename(f, '.*') }
files -= ['layout']
Expand Down
4 changes: 3 additions & 1 deletion test/app_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
class AppTest < Dashing::Test
def setup
@connection = []
app.settings.connections = [@connection]
# stop sinatra from handling Hash value specially which makes it merge new value into previous one
app.settings.connections = nil
app.settings.connections = {@connection => ['some_widget']}
app.settings.auth_token = nil
app.settings.default_dashboard = nil
app.settings.history_file = File.join(Dir.tmpdir, 'history.yml')
Expand Down