search

mendicant-original / newman

Newman: a microframework for building email-based applications.
116 stars 11 forks source link

Server#default_logger doesn't play with Server#simple! #32

Closed practicingruby closed 12 years ago

practicingruby commented 12 years ago

See #31. Basically, default_logger caches the logger object as soon as Server object is initialized, preventing a change made to the debug_mode settings from taking effect if it is made after the object was created but before a tick or run call is made. I could easily move the logger caching code into tick (and it'd be a no-op on subsequent ticks), but that feels nasty. @brentvatne, @ericgj, thoughts?

ericgj commented 12 years ago

Can you give an example of when the service settings would change after the server is initialized? I'm having trouble understanding the problem.

practicingruby commented 12 years ago

@ericgj: Take a look at lib/newman/runner.rb in the commits on #31. If we didn't have this problem, it'd be possible to use Server.simple! and save a lot of manual futzing around.

ericgj commented 12 years ago

Sorry, just seeing that now. What if the logger were configurable either (a) through settings file or (b) from command line options or (c) through runner methods? Rather than a completely separate route into the server via the constructor.

practicingruby commented 12 years ago

@ericgj: That seems okay to me, but it's not clear to me how it'd look. Can you sketch something up? (non-implemented sketches of the API are OK)

ericgj commented 12 years ago

Well, the distinctions between the settings and the runner file and the command line are a bit hazy to me now. Maybe some settings.service.* settings should be in runner file now?

Anyway, maybe there should be a separate set of settings for the logger, (which maybe can be overrode in the runner or command-line), like

settings.logger.file = ...
settings.logger.debug_mode = ...

then in the server,

def logger
  @logger ||=  #  build the logger from settings
end
practicingruby commented 12 years ago

Hmm... I suppose at this point there is little distinction between the settings file and the runner file and it may be worth combining them somehow. However, I would want to be careful to make sure that it wasn't required to use the newman executable to fire up a server, because that would be too aggressive. I think that's somewhat of an unrelated question though.

I like the idea of introducing settings.logger, but I'm not that debug_mode is specific to it, or a service level setting. I think that we may do more than just tweak logger settings in debug mode, so I want to wait and see how that plays out.

Your logger method is roughly equivalent to caching in tick, which is what I mentioned before. I don't want to explicitly reference instance variables anywhere in our code, but something similar could be accomplished with method calls. That's probably what I'll end up doing.

Thanks for the ideas... I'll sleep on this and see what I can come up with.