[jira] [Updated] (SQOOP-3266) Update 3rd party and manual test running related info in COMPILING.txt

[Boglarka Egyed (JIRA)]

 [ 
https://issues.apache.org/jira/browse/SQOOP-3266?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Boglarka Egyed updated SQOOP-3266:
--
Attachment: SQOOP-3266.patch

I forgot to attach the updated patch (which is however attached to the Review 
Request).

> Update 3rd party and manual test running related info in COMPILING.txt
> --
>
> Key: SQOOP-3266
> URL: https://issues.apache.org/jira/browse/SQOOP-3266
> Project: Sqoop
>  Issue Type: Improvement
>  Components: docs
>Affects Versions: 1.4.6
>Reporter: Boglarka Egyed
>Assignee: Boglarka Egyed
>Priority: Trivial
> Fix For: 1.5.0
>
> Attachments: SQOOP-3266.patch, SQOOP-3266.patch
>
>
> MSSQL tests have been added to 3rd part test suite in SQOOP-3174, 
> COMPILING.txt should be updated accordingly. Also, 3rd party test execution 
> on native database servers could be more verbose.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Updated] (SQOOP-3153) Sqoop export with --as- error message could be more verbose

[Ferenc Szabo (JIRA)]

 [ 
https://issues.apache.org/jira/browse/SQOOP-3153?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ferenc Szabo updated SQOOP-3153:

Attachment: SQOOP-3153.patch

> Sqoop export with --as- error message could be more verbose
> -
>
> Key: SQOOP-3153
> URL: https://issues.apache.org/jira/browse/SQOOP-3153
> Project: Sqoop
>  Issue Type: Improvement
>Affects Versions: 1.4.6
>Reporter: Boglarka Egyed
>Assignee: Ferenc Szabo
>Priority: Minor
> Attachments: SQOOP-3153.patch, SQOOP-3153.patch
>
>
> Currently the error message during an export with --as-sequencefile or 
> --as-avrodatafile or --as-parquetfile is the following:
> {code}
> ERROR tool.BaseSqoopTool: Error parsing arguments for export:
> ERROR tool.BaseSqoopTool: Unrecognized argument: --as-avrodatafile
> {code}
> Based on the result of SQOOP-3151 this error message could be more verbose as 
> it is a bit misleading now.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Re: Review Request 64417: SQOOP-3153 Sqoop export with --as- error message could be more verbose

[Fero Szabo via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64417/
---

(Updated Dec. 14, 2017, 1:45 p.m.)


Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.


Changes
---

as discussed with Szabi, I've reorganized imports, as 'Optimize imports on the 
fly' rearranged their order unnecessarily


Bugs: SQOOP-3153
https://issues.apache.org/jira/browse/SQOOP-3153


Repository: sqoop-trunk


Description
---

The errormessages are generated by the hasUnrecognizedArgs funciton, which is 
located in the base class of every Tool: BaseSqoopTool. 

I overrided the hasUnrecognizedArgs function, in the subclass (i.e. the 
ExportTool class), this calls the original function first and then checks 
whether the file formats were specified and prints an additional hint / error 
message.

Example output:
2017-12-07 16:02:42,640 ERROR [main] tool.BaseSqoopTool 
(BaseSqoopTool.java:hasUnrecognizedArgs(335)) - Error parsing arguments for 
export:
2017-12-07 16:02:42,643 ERROR [main] tool.BaseSqoopTool 
(BaseSqoopTool.java:hasUnrecognizedArgs(338)) - Unrecognized argument: 
--as-parquetfile
2017-12-07 16:02:42,643 ERROR [main] tool.ExportTool 
(ExportTool.java:hasUnrecognizedArgs(405)) - Please note that the export tool 
detects the file format automatically and does not support it as an argument: 
--as-parquetfile

Also, corrected a typo in the function javadoc.

Concerns:
- Please let me know if a different message would provide more clarity.
- The intention was to implement the decorator pattern here, please feel free 
to point it out  if a different, simpler design would make more sense.


Diffs (updated)
-

  src/java/org/apache/sqoop/tool/BaseSqoopTool.java 6a4dcb09 
  src/java/org/apache/sqoop/tool/ExportTool.java cd6cdf32 


Diff: https://reviews.apache.org/r/64417/diff/6/

Changes: https://reviews.apache.org/r/64417/diff/5-6/


Testing
---

The following tests passed:
- Unit tests
- 3rd party integration tests

I haven't added a new test case since testHCatExportWithParquetFile in the 
TestHCatalogBasic class already covers the code path and didn't require 
modification.


Thanks,

Fero Szabo

[jira] [Commented] (SQOOP-3266) Update 3rd party and manual test running related info in COMPILING.txt

[Hudson (JIRA)]

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

Hudson commented on SQOOP-3266:
---

SUCCESS: Integrated in Jenkins build Sqoop-hadoop200 #1138 (See 
[https://builds.apache.org/job/Sqoop-hadoop200/1138/])
SQOOP-3266: Update 3rd party and manual test running related info in (vasas: 
[https://git-wip-us.apache.org/repos/asf?p=sqoop.git&a=commit&h=c975dc4c3b76148507e12172f236fa582c0eec93])
* (edit) COMPILING.txt


> Update 3rd party and manual test running related info in COMPILING.txt
> --
>
> Key: SQOOP-3266
> URL: https://issues.apache.org/jira/browse/SQOOP-3266
> Project: Sqoop
>  Issue Type: Improvement
>  Components: docs
>Affects Versions: 1.4.6
>Reporter: Boglarka Egyed
>Assignee: Boglarka Egyed
>Priority: Trivial
> Attachments: SQOOP-3266.patch
>
>
> MSSQL tests have been added to 3rd part test suite in SQOOP-3174, 
> COMPILING.txt should be updated accordingly. Also, 3rd party test execution 
> on native database servers could be more verbose.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (SQOOP-3266) Update 3rd party and manual test running related info in COMPILING.txt

[ASF subversion and git services (JIRA)]

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

ASF subversion and git services commented on SQOOP-3266:


Commit c975dc4c3b76148507e12172f236fa582c0eec93 in sqoop's branch 
refs/heads/trunk from [~vasas]
[ https://git-wip-us.apache.org/repos/asf?p=sqoop.git;h=c975dc4 ]

SQOOP-3266: Update 3rd party and manual test running related info in 
COMPILING.txt

(Boglarka Egyed via Szabolcs Vasas)


> Update 3rd party and manual test running related info in COMPILING.txt
> --
>
> Key: SQOOP-3266
> URL: https://issues.apache.org/jira/browse/SQOOP-3266
> Project: Sqoop
>  Issue Type: Improvement
>  Components: docs
>Affects Versions: 1.4.6
>Reporter: Boglarka Egyed
>Assignee: Boglarka Egyed
>Priority: Trivial
> Attachments: SQOOP-3266.patch
>
>
> MSSQL tests have been added to 3rd part test suite in SQOOP-3174, 
> COMPILING.txt should be updated accordingly. Also, 3rd party test execution 
> on native database servers could be more verbose.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Re: Review Request 64417: SQOOP-3153 Sqoop export with --as- error message could be more verbose

[Fero Szabo via Review Board]

> On Dec. 13, 2017, 9:56 a.m., Szabolcs Vasas wrote:
> > src/java/org/apache/sqoop/tool/ExportTool.java
> > Lines 22 (patched)
> > 
> >
> > This import is unused.

Removed.

I've turned on the 'Optimize imports on the fly' option in my IDE as well, that 
should prevent this from happening in the future.


> On Dec. 13, 2017, 9:56 a.m., Szabolcs Vasas wrote:
> > src/java/org/apache/sqoop/tool/ExportTool.java
> > Lines 51 (patched)
> > 
> >
> > Arrays.asList returns a list which is unmodifiable so 
> > Collections.unmodifiableList is redundant.

Are you sure? That is not what I found here...
https://docs.oracle.com/javase/7/docs/api/java/util/Arrays.html#asList(T...)

I see that most operations (add, remove) are unsupported, but the set(int 
index, String value) function still works, so I'd like to leave this as is.


> On Dec. 13, 2017, 9:56 a.m., Szabolcs Vasas wrote:
> > src/java/org/apache/sqoop/tool/ExportTool.java
> > Lines 403 (patched)
> > 
> >
> > Do we need to convert the array to a list?

Nope, removed.


> On Dec. 13, 2017, 9:56 a.m., Szabolcs Vasas wrote:
> > src/java/org/apache/sqoop/tool/ExportTool.java
> > Lines 406 (patched)
> > 
> >
> > I think you could make this condition more readable by introducing a 
> > method like org.apache.commons.cli.Util#stripLeadingHyphens.

Done.


- Fero


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64417/#review193657
---


On Dec. 13, 2017, 11:19 a.m., Fero Szabo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64417/
> ---
> 
> (Updated Dec. 13, 2017, 11:19 a.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-3153
> https://issues.apache.org/jira/browse/SQOOP-3153
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> The errormessages are generated by the hasUnrecognizedArgs funciton, which is 
> located in the base class of every Tool: BaseSqoopTool. 
> 
> I overrided the hasUnrecognizedArgs function, in the subclass (i.e. the 
> ExportTool class), this calls the original function first and then checks 
> whether the file formats were specified and prints an additional hint / error 
> message.
> 
> Example output:
> 2017-12-07 16:02:42,640 ERROR [main] tool.BaseSqoopTool 
> (BaseSqoopTool.java:hasUnrecognizedArgs(335)) - Error parsing arguments for 
> export:
> 2017-12-07 16:02:42,643 ERROR [main] tool.BaseSqoopTool 
> (BaseSqoopTool.java:hasUnrecognizedArgs(338)) - Unrecognized argument: 
> --as-parquetfile
> 2017-12-07 16:02:42,643 ERROR [main] tool.ExportTool 
> (ExportTool.java:hasUnrecognizedArgs(405)) - Please note that the export tool 
> detects the file format automatically and does not support it as an argument: 
> --as-parquetfile
> 
> Also, corrected a typo in the function javadoc.
> 
> Concerns:
> - Please let me know if a different message would provide more clarity.
> - The intention was to implement the decorator pattern here, please feel free 
> to point it out  if a different, simpler design would make more sense.
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 6a4dcb09 
>   src/java/org/apache/sqoop/tool/ExportTool.java cd6cdf32 
> 
> 
> Diff: https://reviews.apache.org/r/64417/diff/5/
> 
> 
> Testing
> ---
> 
> The following tests passed:
> - Unit tests
> - 3rd party integration tests
> 
> I haven't added a new test case since testHCatExportWithParquetFile in the 
> TestHCatalogBasic class already covers the code path and didn't require 
> modification.
> 
> 
> Thanks,
> 
> Fero Szabo
> 
>

Re: Review Request 64251: SQOOP-3266: Update 3rd party and manual test running related info in COMPILING.txt

[Szabolcs Vasas]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64251/#review193783
---


Ship it!




Ship It!

- Szabolcs Vasas


On Dec. 13, 2017, 10:59 a.m., Boglarka Egyed wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64251/
> ---
> 
> (Updated Dec. 13, 2017, 10:59 a.m.)
> 
> 
> Review request for Sqoop, Fero Szabo and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-3266
> https://issues.apache.org/jira/browse/SQOOP-3266
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> MSSQL tests have been added to 3rd part test suite in SQOOP-3174, 
> COMPILING.txt should be updated accordingly. Also, 3rd party test execution 
> on native database servers could be more verbose.
> 
> 
> Diffs
> -
> 
>   COMPILING.txt f531dd49a77d86b747478b4b8aad1a62d63515af 
> 
> 
> Diff: https://reviews.apache.org/r/64251/diff/3/
> 
> 
> Testing
> ---
> 
> Only text change.
> 
> 
> Thanks,
> 
> Boglarka Egyed
> 
>