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.

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

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

Closed
practicingruby opened this issue Feb 17, 2012 · 6 comments
Closed

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

practicingruby opened this issue Feb 17, 2012 · 6 comments
Labels
Milestone

Comments

@practicingruby
Copy link
Contributor

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
Copy link
Contributor

ericgj commented Feb 17, 2012

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
Copy link
Contributor Author

@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
Copy link
Contributor

ericgj commented Feb 17, 2012

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
Copy link
Contributor Author

@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
Copy link
Contributor

ericgj commented Feb 17, 2012

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants