Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Without autostart #112

Closed
wants to merge 1 commit into from

4 participants

@schmurfy

Here is something concrete to add to the discution :)
I was sure it would be easy but did not expect it to be that easy, this patch allows to do this:

require 'celluloid'
# use celluloid as before, nothing changed

or this:

require 'celluloid/without_autostart'

# do what you want WITHOUT celluloid

Celluloid.start()

# use celluloid as before

And as a bonus you can even call "Celluloid.start()" as many times as you want, the system will only be started once.

So what do you think of this ?

As for the documentation the more I think of it the more I think it should not be documented, if you need this for a good reason you will know where to look.

PS: Github do not shows correctly the file rename, the real diff is really small.

@tarcieri
Owner

I can't exactly say I'm happy with having to massively reorganize the codebase just to facilitate a request I'm uneasy about to begin with. I think there's a lower impact way to do this, like moving all the stuff you don't want to happen at startup into boot.rb and setting some constant that's checked prior to requiring boot.rb

@halorgium
Owner

I don't think there is any reason to move all of the code out of celluloid.rb.
From reading the email thread, it sounds like you just wanted to stop the starting of the default actors.

@schmurfy

@halorgium yes but since requiring celluloid like currently should start those actors how would you do it ?
@tarcieri I understand your feeling but the diff is really not what it looks like in github, it just don't understand that a file has been renamed, what I did was:

  • rename celluloid.rb to without_autostart.rb
  • wrapping everything in boot.rb in a method
  • added a new celluloid.rb file which require the renamed file and call the newly created method

I will give the constant idea a try tomorrow if you prefer.

@halorgium
Owner

The const idea is similar to the one added to Rake a while back.

@schmurfy

here is another lighter attempt :)

@halorgium
Owner

The new method on the Celluloid module means that all actors will now have this method by default.
Can you switch it to a module method explicitly?

@tarcieri
Owner

I think a better way to do this is to put everything that starts on boot into boot.rb, then selectively require that based on CELLULOID_DISABLE_AUTOSTART

@halorgium
Owner

Yer, this makes sense.
Then you can simply require 'celluloid/boot' to get the original behaviour.

@schmurfy

and it would lead to this when you need to start celluloid later:

def do_something_with_actors
  require 'celluloid/boot'

end

very ugly :s

@tarcieri
Owner

Why is that "very ugly" and how would it be any less ugly from what you're proposing?

@schmurfy
Celluloid.start()

vs

require 'celluloid/boot'

Maybe that is just me but adding a require in a function makes me sad.

@DrPheltRight

Not sure of the implementation of this feature but I'd like to be able to use Celluloid::Future without booting up the actor singleton. In fact, I don't really like libraries that force this kind of singleton via a require statement usually anyway.

@schmurfy

You will not be able to use futures without a running celluloid system, what would you want to do with it ?

@schmurfy

let's forget this, it does not look like we will get anything from it.

@schmurfy schmurfy closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 5, 2012
  1. @schmurfy
This page is out of date. Refresh to see the latest.
Showing with 20 additions and 7 deletions.
  1. +20 −7 lib/celluloid/boot.rb
View
27 lib/celluloid/boot.rb
@@ -1,10 +1,23 @@
# Things to run after Celluloid is fully loaded
+module Celluloid
+ module_function
+
+ def start
+ unless @started
+ # Configure default systemwide settings
+ Celluloid.task_class = Celluloid::TaskFiber
+ Celluloid.logger = ::Logger.new(STDERR)
+
+ # Launch default services
+ # FIXME: We should set up the supervision hierarchy here
+ Celluloid::Notifications::Fanout.supervise_as :notifications_fanout
+ Celluloid::IncidentReporter.supervise_as :default_incident_reporter, STDERR
+ @started = true
+ end
+ end
+end
-# Configure default systemwide settings
-Celluloid.task_class = Celluloid::TaskFiber
-Celluloid.logger = Logger.new(STDERR)
-# Launch default services
-# FIXME: We should set up the supervision hierarchy here
-Celluloid::Notifications::Fanout.supervise_as :notifications_fanout
-Celluloid::IncidentReporter.supervise_as :default_incident_reporter, STDERR
+unless defined?(CELLULOID_DISABLE_AUTOSTART)
+ Celluloid.start()
+end
Something went wrong with that request. Please try again.