Re: [hudson-dev] [Hudson-Dev] Re: Questions and ideas for JSR330 plugins

[Henrik Lynggaard Hansen <henrik@xxxxxxx>]

2012/1/30 Jason Dillon <jason@xxxxxxxxxxxx>:
>> - show me where I can write files (provide a "File-like" object
>> pointing to a directory)
>
> IIRC there isn't really a standard for where plugins put their files (for better or worse).   I guess it would be nice to have a standard, though I'd almost rather have a data abstraction layer... though that could get messy.

You are correct that there isn't a standard, and that for me is a
problem. This unstructured placement of files makes it harder to
manage a hudson instance e.g. I don't know which files to backup or
more, and in general looks very bad.

Therefore I think we should give plugin developers a isolated place to
write their files (in cases where they aren't part of the normal
serialization) e.g. a directory per plugin or simply a work directory.
Injecting this instance should provide a pointer to such area.

Maybe a data abstraction layer is a better thing, but I would prefer
that we start small to get the quick benefit, and then we can add
features later if needed.


>> - show me the Hudson version, and maybe other metadata like hudson home
>> - show me which other plugins exists on this instance
>
> Seems like the PluginManager would be the component to inject to get this information.

If there is already a place for this then there is of course no need
to add this to a potential PluginContext. However given the state of
many of the current APIs, the next question then becomes. Should we
create a "clean" PluginManager API or is the current one in good
enough quality. On one hand I hate the double work of creating a
wrapper, but on the other hand I really like to shield new plugins
from the worst parts of Hudson.
>
> So there isn't much difference between:
>
> hudson.plugin.jobrevision=DEBUG
>
> and:
>
> com.thalesgroup.hudson.plugins.jobrevision=DEBUG
>

Depends on who you are. For us or for the plugin developer there is
very little difference. But for a user there is a great difference in
how easy it is for them to change the logging of a plugin. The first
one is much easier to discover. Instead of just talking about
injection let us take a step back.

Let us assume  I am a hudson user who has just installed a plugin in
Hudson (e.g. jobrevision)  and it fails for some reason. I now want to
enable more logging for that plugin. I can either
A) on a help text in the logging page (http://localhost:8080/log) it
is documented that all modern plugins follow the standard
"hudson.plugin.<plugin name>" and I give it a try with
hudson.plugin.jobrevision, which works
B) somehow discover that the correct logger is
"com.thalesgroup.hudson.plugins.jobrevision", hopefully by finding it
on the plugin documentation page, or by enabling all loggers.

Surely we can agree that easy to discover logger names makes things
easier for users...The next question then becomes how do we ensure
easy to discover logger names. There are several options, some of
which are

A) Provide the plugin developer with a logger instance already
configured the use the correct name
B) Document it as part of a plugin developer best practice and hope
plugin developers follow it
C) Provide some sort of mapping between human readable names, and the
logger names (if I read your suggestion right)
D) ??

Without looking at how things would be coded, option A does look
appealing since it automatically ensures compliance

> If what you want is to provide a better way to configure logging, then it may be better to make a component which exposes mapping from logger name to human friendly name/description and then present that to the user to flip on/off vs. using a single injected logger for all plugin logging.  The single logger name for complex plugins will lose significant detail as to where the logging is coming from.

You are correct that wtih option A does not allow for having different
loggers within a plugin, which could lead to information loss
depending on the logging format. Some log formats prints the source
method and line numbers, some don't

What would your suggestion be for making sure users have easy
discoverable logger names?

>> I did not know that I could inject the extensions, so thank you for
>> the explanation. So it looks like
>> http://wiki.hudson-ci.org/display/HUDSON/Extension+points is the best
>> documentation for such an overview although it doesn't explain how to
>> inject stuff like the document storage used by maven 3 plugins.
>
> Sure, that page doesn't cover components exposed by every possible plugin... that would be a difficult list to grok.

Agreed but we should have  page that describes exposed components from
the core and possibly plugins bundled with the core.
> private final DocumentManager documentManager;
<snip>
> ... to get a reference to the component which handles document management (preferred over the store, which is just an abstraction over the persistence).  This is setter injection, ctor injection works too.  Field injection should also work but I'd generally avoid using that.
>
Thank you.

> But, back to your point... perhaps plugins should be providing documentation about their components?

If they expose something to the public they should. Is there a way to
distinguish between publicly and privately exposed components?


> Well, you can look for thinks that have @Named on them, or @ImplementedBy... or @ExtensionPoint.  Perhaps there needs to be a extension point documentation generator thingy which the hpi packaging can invoke and spit out documentation or something.

If we somehow can automate the generation of API (not javadoc)
documentation it would be great. Hopefully the core parts should be
stable enough that they can be manually documented?

>>
>> if (Hudson.getInstance().getPlugin != null) {
>>  ClaimCause cause =.....
>> }
>
> I think this is still in the realm of inject to the PluginManager and ask it for these details, vs. exposing this on a generic PluginContext.  Not ever plugin will want/care to know about this information.  I think it will be hard to separate what is generic enough to be part of the PluginContext api vs. what should be separate components.  There may still be some valid reason to make a PluginContext component, but I don't see it yet.  You already have Plugin impls which you can inject and you could make a support class to put whatever common bits you want in and inject it as needed.

See my comments above about the PluginManager

>> So the functionality is essentially there, so this idea was more to
>> chip away at the god object.
>
> Its defs good to get away from the evil god object, but I don't think introducing PluginContext really helps all that much vs. exposing details into services and then injecting the services when you need the functionality they provide.

fair enough.

> My recommendation would be to
> a) don't inject loggers (or init them from some injected context object), use loggers as I described above
> b) implement a component to expose the details for human configuration ease (if you really want/need this) or just use the base-package to expose to humans

If we are not injecting them, we should agree on a alternative for
ensuring easy to discover logger names or easy log configuration
either using documentation or by some GUI enhancement.

For a simple solution we could document that the best practice is to
have a "base name" as "hudson.plugin.<plugin name>", and then they are
free to add postfixes to that name.

> c) use slf4j over creating yet another logging facade

Side note: It wouldn't be my favourite facade. why has no logging
facade discovered var-args, instead we get some crap 4 overloaded
methods for the same, and I still have to manually create Object
arrays.

But the facade should be chosen for its technical merits not my
personal opinion ;-) As long as it is clearly documented so plugin
developers don't use JUL or include a log4j

> d) make services to expose commonly used functionality/details and have plugins inject to access them

agreed

> e) punt for now on making a PluginContext until there is a clear need for it (hudson version, plugins, directory, etc) doesn't seem like a good reason to introduce this to me

Since you have shown the other information is available in other
injectable objects, I agree.

However I still think there is value in creating a component for
"where to safely write files"

Best regards
Henrik