[jira] [Comment Edited] (HADOOP-12341) patch file confuses test-patch (date format problems)

["Allen Wittenauer (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-12341?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14733984#comment-14733984 ] 

Allen Wittenauer edited comment on HADOOP-12341 at 9/7/15 6:43 PM:
-------------------------------------------------------------------

bq. test-patch had skipped file type guessing if the file has .patch suffix, but this behaviour is changed on HADOOP-12129. Is there any concerns if we return this behaviour back?

The original code parsed the HTML with the assumption that JIRA (well, users) wouldn't lie and that users would always name the attachment with .patch .  This isn't necessarily a safe assumption to make anymore, for a few reasons:
* Users are users and will do everything you don't expect them to do.
* This doesn't help with patch files that don't end in .patch (e.g., .diff, .txt, etc)
* In non-JIRA systems, what exactly can the code look at?
* In non-ASF JIRA, do we know what their convention will be?

I thought about making another callback into the plugin to basically ask it "do you, plugin, guarantee this is a patch file?"  but that doesn't seem like very defensive programming: what if the plugin can't know? Plus, we directly take user input on the command line for patch files, so we would still need to write code that verifies a patch file is actually a patch file.  So the plugin verification seemed rather pointless.... if test-patch can validate user input, then validating plugin patch files should work just as well.

Thus that code was removed.


was (Author: aw):
bq. test-patch had skipped file type guessing if the file has .patch suffix, but this behaviour is changed on HADOOP-12129. Is there any concerns if we return this behaviour back?

The original code parsed the HTML with the assumption that JIRA (well, users) wouldn't lie and that users would always name the attachment with .patch .  This isn't necessarily a safe assumption to make anymore, for a few reasons:
* Users are users and will do everything you don't expect them to do.
* In non-JIRA systems, what exactly can the code look at?
* In non-ASF JIRA, do we know what their convention will be?

I thought about making another callback into the plugin to basically ask it "do you, plugin, guarantee this is a patch file?"  but that doesn't seem like very defensive programming: what if the plugin can't know? Plus, we directly take user input on the command line for patch files, so we would still need to write code that verifies a patch file is actually a patch file.  So the plugin verification seemed rather pointless.... if test-patch can validate user input, then validating plugin patch files should work just as well.

Thus that code was removed.

> patch file confuses test-patch (date format problems)
> -----------------------------------------------------
>
>                 Key: HADOOP-12341
>                 URL: https://issues.apache.org/jira/browse/HADOOP-12341
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: yetus
>    Affects Versions: HADOOP-12111
>            Reporter: Allen Wittenauer
>         Attachments: HADOOP-12326.002.patch, HADOOP-12375.HADOOP-12111.02.patch
>
>
> This was attached to HADOOP-12326 .



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)