ProcessMonitor was made private braking it's previously "public" contract

0 views
Skip to first unread message

Steve Trefethen

unread,
Jan 27, 2009, 1:21:28 AM1/27/09
to ccnet-devel
The class ProcessMonitor has been made private which breaks code that
relies on that class. In my situation I need to specifically control
an executable and due to the lack of extensibility of ExecutableTask
the only option I felt I had was to copy the entire ExecutableTask
class and make the necessary changes. This class references what was a
public ProcessMonitor class with a method called
GetProcessMonitorByProject.

The class is now private and the method is gone. This puts me in a
very awkward position of having to reimplement the Task class
hierarchy I've created to deal with the shortcomings of
ExecutableTask. I'd debated about submitting a patch for my changes
but was concerned that my case may be too specific and now I'm rather
stuck. It appears the implementation of ProcessExecutor has undergone
many changes so I'll need to understand those first.

I think it's really unfortunate the public "contract" of that class
was been changed as it means a lot more work for me at this point when
I had a working solution.

-Steve

David Cameron

unread,
Jan 27, 2009, 3:14:46 AM1/27/09
to ccnet...@googlegroups.com
Hi Steve

I would be happy to hear about your specific requirement and the patch
you implemented to meet the requirement.

I can understand your frustration. It's difficult to know what parts
of the public interfaces people are depending on. Certainly there
aren't any hooks in CC.Net that are designed for extensibility, and I
hadn't considered that others might have tasks built around
ProcessMonitor.

I'm facing a tricky balance between maintainability and consistency of
the code base, and avoiding breaking third-party extensions or
customizations. A lot of changes have been made to the code base by
applying patches that weren't necessarily written with a view of the
whole code base. Some of those changes, including earlier changes to
ProcessExecutor and ProcessMonitor, introduced bugs. It was in the
process of trying to understand those changes and fix the bugs that I
made ProcessMonitor private.

PorcessExecutor and ProcessMonitor are sensitive to concurrency
concerns, and that is the reason I wanted to make more of that code
private.

Could we talk about the shortcomings of ExecutableTask that you've
encountered or the specific changes that you need to make? The best
way to ensure compatibility of your changes is to get them included in
the mainline.

Dave Cameron
CruiseControl.NET - http://ccnet.thoughtworks.com

Steve Trefethen

unread,
Jan 27, 2009, 8:18:03 PM1/27/09
to ccnet-devel
Hi Dave,
Thanks very much for the detailed response. I do understand the
problem and unfortunately I wasn't in a great situation to assemble a
real patch due to time pressures on my project. I'm reviewing the code
now (not finished yet) and at least two of the major issues, lack of
access to GetProcessArguments and non-virtual Run method have been
resolved. This looks much better than the previous implementation so
the work is _very_ much appreciated.

I'm working on updating my code now and if I run into any other issues
I'll be sure to let you know.

Thanks again for your reply,
-Steve
http://www.stevetrefethen.com/blog/
> CruiseControl.NET -http://ccnet.thoughtworks.com

Steve Trefethen

unread,
Jan 28, 2009, 11:48:09 PM1/28/09
to ccnet-devel
Hi Dave,
So, I've had a chance to go over the code more and I'd like to say
nice work! The items I need have been implemented which is great!

The only minor suggestion would be to provide a class that does not
add the ReflectorProperty attributes to all of the properties. For
example, I don't want the "executable" property to be required since,
in my case, it's always the same .EXE. I've created a task class that
"knows" how to run a specific application.

Basically, the change would be to ExecutableTask -> ExecutableBase and
remove the ReflectorProperty attrbutes and create an ExecutableTask
that adds the attributes as necessary.

Fwiw, my custom task config looks like this:

<editask>
<buildTimeoutSeconds>300</buildTimeoutSeconds>
<job>iTrade Inbound</job>
</editask>

Which is nice and concise and doesn't require the need to specify
<executable> a dozen times.

Again, nice job, this is a lot more extensible than the previous
implementation.
-Steve

On Jan 27, 12:14 am, David Cameron <dave...@gmail.com> wrote:
> CruiseControl.NET -http://ccnet.thoughtworks.com

Steve Trefethen

unread,
Jan 28, 2009, 11:51:38 PM1/28/09
to ccnet-devel
Hi Dave,
One more note. I can totally work with the existing changes so
please read my last post a more of a minor suggestion. I wanted to
clarify in case it didn't come across that way.

-Steve
Reply all
Reply to author
Forward
0 new messages