You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Neeme Praks <ne...@apache.org> on 2005/05/04 20:19:05 UTC

[patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Hi!

Attached is a patch for the optional FTP task. It addresses the 
following issues:
1. support for new features in upcoming commons-net 1.4.0 release:
     * possibility to customize date format parsing
     * possibility to specify server timezone
     * * possibility to specify custom month name strings
Example:
<ftp>
...
<configuration recentDateFormatStr="yyyy-MM-dd HH:mm" 
defaultDateFormatStr="yyyy-MM-dd HH:mm" serverTimeZoneId="Europe/Oslo" 
shortMonthNames="01|02|03|04|05|06|07|08|09|10|11|12"/>
...
</ftp>

Currently this is implemented as a child element, to make the 
implementation "modular". However, as there is no real benefit in adding 
more than one configuration element, it could also be turned into 
attributes directly on the <ftp> task.

2. this part of the patch addresses the issue brought up earlier by 
Pierre Grimaud (pierre_grimaud@hotmail.com): the speed of FTP task in 
the case of long directory listings. I sped it up by listing the 
(parent) directories instead of individual files and caching the listings.

Currently it lacks the configuration option to turn this functionality 
off, as I was not sure if this option is needed and if it should be on 
or off by default. Trivial to add, anyway.


Both of these changes do not have any unit tests as after having a 
glance at the FTP task unit test it seemed quite complex and I figured 
that maybe it is easier for you to add those tests. I guess adding tests 
is relevant only for the new functionality.

Also, I didn't include patches for documentation as I'm not sure if this 
implementation is final or if you would like to refactor it into some 
other form.

Let me know what you think of this all!

Rgds,
Neeme

Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by si...@insession.com.
While you are at it, could you please also incorporate my simple patch to 
the FTP task? 

http://issues.apache.org/bugzilla/show_bug.cgi?id=34853

It would be nice if it also could make it into the 1.6.4 release.

Thanks,

John

This e-mail message and any attachments may contain confidential, 
proprietary or non-public information.  This information is intended 
solely for the designated recipient(s).  If an addressing or transmission 
error has misdirected this e-mail, please notify the sender immediately 
and destroy this e-mail.  Any review, dissemination, use or reliance upon 
this information by unintended recipients is prohibited.  Any opinions 
expressed in this e-mail are those of the author personally.

Stefan Bodewig <bo...@apache.org> wrote on 11/05/2005 05:45:23 PM:

> On Tue, 10 May 2005, Steve Cohen <sc...@javactivity.org> wrote:
> 
> > As the author of the commons-net code that Mr. Praks is relying on,
> > and an Ant committer, I would be happy to take a look at his code
> > and "sponsor" it for the 1.6.4 release.
> 
> There is no need to sponsor, you are a committer.  We usually work in
> commit-then-review mode, at least for CVS HEAD, so go ahead and commit
> it to HEAD.
> 
> After you've done that, you could call for a vote for merging the
> change over to the 1.6 branch.  Personally I don't think a vote would
> be necessary, though.
> 
> Stefan
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
> 

Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Steve Cohen <sc...@javactivity.org>.
Stefan Bodewig wrote:
> On Tue, 10 May 2005, Steve Cohen <sc...@javactivity.org> wrote:
> 
> 
>>As the author of the commons-net code that Mr. Praks is relying on,
>>and an Ant committer, I would be happy to take a look at his code
>>and "sponsor" it for the 1.6.4 release.
> 
> 
> There is no need to sponsor, you are a committer.  We usually work in
> commit-then-review mode, at least for CVS HEAD, so go ahead and commit
> it to HEAD.
> 
> After you've done that, you could call for a vote for merging the
> change over to the 1.6 branch.  Personally I don't think a vote would
> be necessary, though.
> 
> Stefan
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
> 
> 
> 
I may be a committer but Mr Praks is not.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Stefan Bodewig <bo...@apache.org>.
On Tue, 10 May 2005, Steve Cohen <sc...@javactivity.org> wrote:

> As the author of the commons-net code that Mr. Praks is relying on,
> and an Ant committer, I would be happy to take a look at his code
> and "sponsor" it for the 1.6.4 release.

There is no need to sponsor, you are a committer.  We usually work in
commit-then-review mode, at least for CVS HEAD, so go ahead and commit
it to HEAD.

After you've done that, you could call for a vote for merging the
change over to the 1.6 branch.  Personally I don't think a vote would
be necessary, though.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Stefan Bodewig <bo...@apache.org>.
On Wed, 11 May 2005, Steve Cohen <sc...@javactivity.org> wrote:

> So commit to head as we see fit

yes.

> and maybe it will be moved to the branch?

call for a vote, and do so in time, usually votes are supposed to run
for a week.  We had a vote on 1.6.4 for the current feature set, we
would have to vote on the release again if the code changed.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Steve Cohen <sc...@javactivity.org>.
Stefan Bodewig wrote:
> On Wed, 11 May 2005, Steve Cohen <sc...@javactivity.org> wrote:
> 
> 
>>Other parts of Mr. Praks' patch are less simple (retry handler,
>>etc.) and I've asked him to remove those for now.  These definitely
>>belong in CVS_HEAD after the release.
> 
> 
> The release comes from a branch, so you can go ahead and commit to
> HEAD whenever you see fit.
> 
> Stefan
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
> 
> 
> 

So commit to head as we see fit and maybe it will be moved to the branch?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Stefan Bodewig <bo...@apache.org>.
On Wed, 11 May 2005, Steve Cohen <sc...@javactivity.org> wrote:

> Other parts of Mr. Praks' patch are less simple (retry handler,
> etc.) and I've asked him to remove those for now.  These definitely
> belong in CVS_HEAD after the release.

The release comes from a branch, so you can go ahead and commit to
HEAD whenever you see fit.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Steve Cohen <sc...@javactivity.org>.
Neeme Praks wrote:
> 
> Steve Loughran wrote:
> 
>>
>> I worry about releasing any change to code without giving it time to 
>> stabilise and beta test. Last minute "this won't break anything" 
>> patches always break something. Always. At least in my experience.
>>
>> If commons1.4.0 is incompatible with shipping <ftp> then yes, we have 
>> no choice but to upgrade. But if it is a feature enhancement, then it 
>> needs
>> to go into CVS_HEAD
> 
> 
> Very legitimate concern.
> However, this is a trivial change.
> commons-net 1.4.0 added a configuration javabean for FTP client.
> It is a simple value-object that has some setters and then it can be 
> used to configure a FTP client instance.
> 
> All the added code does is expose this javabean on the FTP task through 
> delegating setters.
> 
> Let me illustrate with some code:
> 
> public class FTP extends Task {
> [...]
>     private FTPClientConfig configuration = null;
> 
>     /**
>      * Gets a FTPClientConfig. If the configuration object has not been
>      * created yet, it is created also.
>      */
>     private FTPClientConfig getConfiguration() {
>         if (this.configuration == null) {
>             this.configuration = new FTPClientConfig();
>         }
>         return this.configuration;
>     }
> 
>     /**
>      * Delegate method for 
> <code>FTPClientConfig.setDefaultDateFormatStr(String)</code>.
>      * @param defaultDateFormatStr
>      * @see #getConfiguration()
>      * @see FTPClientConfig
>      */
>     public void setDefaultDateFormat(String defaultDateFormatStr) {
>         getConfiguration()
>             .setDefaultDateFormatStr(defaultDateFormatStr);
>     }
> 
> [...there are 5 more delegating setters like this, but I'm skipping them 
> here for clarity...]
> 
>     public void execute() throws BuildException {
>         [...]
>         ftp = new FTPClient();
>         if (this.configuration != null) {
>             ftp.configure(this.configuration);
>         }
>         ftp.connect(server, port);
>         [...]
>     }
> 
> [...]
> }
> 
> Simple enough, no?
> Assuming that commons-net code is bug-free, this code cannot possibly 
> break anything. And it is backwards compatible, if there is no 
> configuration set, no configuration is used.
> 
> Rgds,
> Neeme
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
> 
> 
> 
Well, you're both right.  The new commons-net code is not incompatible 
with Ant.  This is just a new feature of commons-net.

However, it's an extremely simple feature, just passing bean attributes 
around.  I am quite confident that we can add just the support for the 
new commons-net feature and have all the tests pass.

Other parts of Mr. Praks' patch are less simple (retry handler, etc.) 
and I've asked him to remove those for now.  These definitely belong in 
CVS_HEAD after the release.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Steve Loughran <st...@apache.org>.
Neeme Praks wrote:
> 
> Steve Loughran wrote:
> 
>>
>> I worry about releasing any change to code without giving it time to 
>> stabilise and beta test. Last minute "this won't break anything" 
>> patches always break something. Always. At least in my experience.
>>
>> If commons1.4.0 is incompatible with shipping <ftp> then yes, we have 
>> no choice but to upgrade. But if it is a feature enhancement, then it 
>> needs
>> to go into CVS_HEAD
> 
> 
> Very legitimate concern.
> However, this is a trivial change.

so are all changes that end up breaking things.

Ant1.6.4 is really Ant1.6.3a; a late fix for stuff that wasnt caught in 
  the beta test. I am really, really, reluctant to do anything that 
could break stuff. If ant1,6.4 ends up broken, we have to release an 
Ant1.6.5 two weeks later, there is more pressure for last minute 
changes, we introduce new bugs, never stabilise, get a bad reputation 
for release management, etc, etc.

If you look at the commit log, you can see that I tend to avoid 
committing my own changes to the 1.6.x branch either, because I like to 
work with things myself for a bit. Once something is in the public API 
of Ant, we cant remove it. So we need to get it right, And that means 
being strict about last minue commitments. Indeed, there are three 
things I committed that I think might want to go into the 1.6 branch, 
but but I'm not going to do it

  -java1.5 class passthrough changes to JavaEnvUtils
  -java1.5 settings of default rmi compiler options
  -that switch to WeakHashMap.

Why am I not committing them? Because nobody is complaining about them, 
even though they are clearly bugs with trivial fixes.

-Steve

(NB, I'm not the release manager, but note that I'm fairly relaxed about 
changes compared to someone I've worked with. Ruthlessness is a 
wonderful thing when it is time to get something out the door)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Neeme Praks <ne...@apache.org>.
Steve Loughran wrote:
> 
> I worry about releasing any change to code without giving it time to 
> stabilise and beta test. Last minute "this won't break anything" patches 
> always break something. Always. At least in my experience.
> 
> If commons1.4.0 is incompatible with shipping <ftp> then yes, we have no 
> choice but to upgrade. But if it is a feature enhancement, then it needs
> to go into CVS_HEAD

Very legitimate concern.
However, this is a trivial change.
commons-net 1.4.0 added a configuration javabean for FTP client.
It is a simple value-object that has some setters and then it can be 
used to configure a FTP client instance.

All the added code does is expose this javabean on the FTP task through 
delegating setters.

Let me illustrate with some code:

public class FTP extends Task {
[...]
     private FTPClientConfig configuration = null;

     /**
      * Gets a FTPClientConfig. If the configuration object has not been
      * created yet, it is created also.
      */
     private FTPClientConfig getConfiguration() {
         if (this.configuration == null) {
             this.configuration = new FTPClientConfig();
         }
         return this.configuration;
     }

     /**
      * Delegate method for 
<code>FTPClientConfig.setDefaultDateFormatStr(String)</code>.
      * @param defaultDateFormatStr
      * @see #getConfiguration()
      * @see FTPClientConfig
      */
     public void setDefaultDateFormat(String defaultDateFormatStr) {
         getConfiguration()
             .setDefaultDateFormatStr(defaultDateFormatStr);
     }

[...there are 5 more delegating setters like this, but I'm skipping them 
here for clarity...]

     public void execute() throws BuildException {
         [...]
         ftp = new FTPClient();
         if (this.configuration != null) {
             ftp.configure(this.configuration);
         }
         ftp.connect(server, port);
         [...]
     }

[...]
}

Simple enough, no?
Assuming that commons-net code is bug-free, this code cannot possibly 
break anything. And it is backwards compatible, if there is no 
configuration set, no configuration is used.

Rgds,
Neeme


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Steve Loughran <st...@apache.org>.
Steve Cohen wrote:

> Since there is apparently an Ant 1.6.4 version coming out on May 19th 
> and since Neeme Praks has already submitted a patch to accomodate it, 
> why not try to get this into that release?  As the author of the 
> commons-net code that Mr. Praks is relying on, and an Ant committer, I 
> would be happy to take a look at his code and "sponsor" it for the 1.6.4 
> release.
> 
> I realize that there may be other deadlines here.  What do other Ant 
> committers think?

I worry about releasing any change to code without giving it time to 
stabilise and beta test. Last minute "this won't break anything" patches 
always break something. Always. At least in my experience.

If commons1.4.0 is incompatible with shipping <ftp> then yes, we have no 
choice but to upgrade. But if it is a feature enhancement, then it needs
to go into CVS_HEAD

-steve

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Steve Cohen <sc...@javactivity.org>.
Neeme Praks wrote:
> Ok, commons-net 1.4.0 has been released now.
> How can we proceed?
> 
> Meanwhile I also implemented the feature of retry-on-IOException, for 
> FTP task.
> 
> Stefan Bodewig wrote:
> 
>>
>>> The vote has passed and release is being prepared (see below).
>>> Hopefully it will be ready in a day or two.
>>
>>
>> Sounds good.
> 
> 
> -------- Original Message --------
> Subject:     [ANNOUNCEMENT] Commons-Net 1.4.0 Released
> Date:     Tue, 10 May 2005 09:22:31 +0100
> From:     Rory Winston <rw...@eircom.net>
> To:     announcements@jakarta.apache.org, 
> commons-dev@jakarta.apache.org, commons-user@jakarta.apache.org
> 
> 
> The Commons-Net team are pleased to announce the release of version 
> 1.4.0. This release provides several fixes and enhancements, including:
> 
>  - The addition of a new configuration mechanism that enables the 
> FTPClient component to work with a much larger range of server formats 
> and locales;
>  - The addition of missing NTP unit tests;
>  - The addition of a new FTP parser implementation for MVS;
>  - Various fixes to the TFPClient and NTPClient components
> 
> A list of changes can be found at 
> http://jakarta.apache.org/commons/net/changes-report.html#1_4_0
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
> 
> 
> 

Since there is apparently an Ant 1.6.4 version coming out on May 19th 
and since Neeme Praks has already submitted a patch to accomodate it, 
why not try to get this into that release?  As the author of the 
commons-net code that Mr. Praks is relying on, and an Ant committer, I 
would be happy to take a look at his code and "sponsor" it for the 1.6.4 
release.

I realize that there may be other deadlines here.  What do other Ant 
committers think?

Steve Cohen

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Neeme Praks <ne...@apache.org>.
Ok, commons-net 1.4.0 has been released now.
How can we proceed?

Meanwhile I also implemented the feature of retry-on-IOException, for 
FTP task.

Stefan Bodewig wrote:
> 
>>The vote has passed and release is being prepared (see below).
>>Hopefully it will be ready in a day or two.
> 
> Sounds good.

-------- Original Message --------
Subject: 	[ANNOUNCEMENT] Commons-Net 1.4.0 Released
Date: 	Tue, 10 May 2005 09:22:31 +0100
From: 	Rory Winston <rw...@eircom.net>
To: 	announcements@jakarta.apache.org, commons-dev@jakarta.apache.org, 
commons-user@jakarta.apache.org


The Commons-Net team are pleased to announce the release of version 
1.4.0. This release provides several fixes and enhancements, including:

  - The addition of a new configuration mechanism that enables the 
FTPClient component to work with a much larger range of server formats 
and locales;
  - The addition of missing NTP unit tests;
  - The addition of a new FTP parser implementation for MVS;
  - Various fixes to the TFPClient and NTPClient components

A list of changes can be found at 
http://jakarta.apache.org/commons/net/changes-report.html#1_4_0



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Stefan Bodewig <bo...@apache.org>.
On Mon, 09 May 2005, Neeme Praks <ne...@apache.org> wrote:
> Stefan Bodewig wrote:
>> "upcoming" means, it hasn't been released yet, correct?  Is there a
>> release plan for 1.4.0?  We can't make a task as much-used as ftp
>> (judging from the number of bug-reports) depend on a CVS snapshot
>> of an external libary - at least no released version of that task.
> 
> The vote has passed and release is being prepared (see below).
> Hopefully it will be ready in a day or two.

Sounds good.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Neeme Praks <ne...@apache.org>.
Stefan Bodewig wrote:
> 
> "upcoming" means, it hasn't been released yet, correct?  Is there a
> release plan for 1.4.0?  We can't make a task as much-used as ftp
> (judging from the number of bug-reports) depend on a CVS snapshot of
> an external libary - at least no released version of that task.

The vote has passed and release is being prepared (see below).
Hopefully it will be ready in a day or two.

Rgds,
Neeme


-------- Original Message --------
Subject: 	[RESULT] Release Commons-Net 1.4
Date: 	Sat, 7 May 2005 17:16:08 +0100
From: 	Rory Winston <rw...@eircom.net>
To: 	Jakarta Commons Developers List <co...@jakarta.apache.org>
CC: 	pmc@jakarta.apache.org


The vote to release Commons-Net 1.4 has been passed, with 4 binding +1s.

I'll get on the release straight away.

Rory


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement

Posted by Stefan Bodewig <bo...@apache.org>.
On Wed, 04 May 2005, Neeme Praks <ne...@apache.org> wrote:

> 1. support for new features in upcoming commons-net 1.4.0 release:

"upcoming" means, it hasn't been released yet, correct?  Is there a
release plan for 1.4.0?  We can't make a task as much-used as ftp
(judging from the number of bug-reports) depend on a CVS snapshot of
an external libary - at least no released version of that task.

> possibility to customize date format parsing ...

sounds very useful and will fix lots of issues.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org