You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by kinow <gi...@git.apache.org> on 2016/11/06 10:39:22 UTC

[GitHub] jena pull request #188: JENA-1197 return exit code 1 when there is a problem...

GitHub user kinow opened a pull request:

    https://github.com/apache/jena/pull/188

    JENA-1197 return exit code 1 when there is a problem validating a file with riot

    A pull request with a simple approach for JENA-1197. When validate is checked, errors parsing a file will also be reported back, through a `CmdException`.
    
    Quite hard to write a unit test for this, so did some simple regression test locally.
    
    ```
    kinow@localhost:~/Development/java/jena/jena/apache-jena/bin$ JENA_HOME=/home/kinow/Desktop/JENA/apache-jena-3.1.1 ./riot --validate /tmp/invalid.ttl ; echo $?
    23:34:46 ERROR riot                 :: [line: 1, col: 1 ] Out of place: [KEYWORD:I]
    0
    kinow@localhost:~/Development/java/jena/jena/apache-jena/bin$ JENA_HOME=../target/apache-jena-3.1.2-SNAPSHOT ./riot --validate /tmp/invalid.ttl ; echo $?
    23:34:55 ERROR riot                 :: [line: 1, col: 1 ] Out of place: [KEYWORD:I]
    1
    kinow@localhost:~/Development/java/jena/jena/apache-jena/bin$ JENA_HOME=/home/kinow/Desktop/JENA/apache-jena-3.1.1 ./riot --validate ../../jena-sdb/testing/Assembler/graph-assembler.ttl ; echo $?
    0
    kinow@localhost:~/Development/java/jena/jena/apache-jena/bin$ JENA_HOME=../target/apache-jena-3.1.2-SNAPSHOT ./riot --validate ../../jena-sdb/testing/Assembler/graph-assembler.ttl ; echo $?
    0
    ```
    
    For invalid files, the new code prints the same as before, but exists with 1, rather than 0. For valid files, the behavior is consistent too.
    
    Sending a pull request as we are in the middle of a release :-)
    
    Cheers
    Bruno


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kinow/jena JENA-1197

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/jena/pull/188.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #188
    
----
commit 79943f05f4e9040da954f26f22113af9ff36f33f
Author: Bruno P. Kinoshita <br...@yahoo.com.br>
Date:   2016-11-06T10:33:59Z

    JENA-1197 return exit code 1 when there is a problem validating a file with riot

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request #188: JENA-1197 return exit code 1 when there is a problem...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/jena/pull/188


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #188: JENA-1197 return exit code 1 when there is a problem valida...

Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:

    https://github.com/apache/jena/pull/188
  
    Oh, goodness no-- I just used `n` because I'm imagining a series of exit codes (e.g. 0 = success, 3 = partial success (tuples dropped for bad URIs, see STDERR for details), 4 = partial success (tuples dropped for bad syntax, see STDERR for details), etc.) None of which do I have actually planned or am I ready to write, so I guess I'm done talking. :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #188: JENA-1197 return exit code 1 when there is a problem valida...

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:

    https://github.com/apache/jena/pull/188
  
    As long as that `n` is not a count! Exit codes are "enums", not a sneaky way to return a variable result.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #188: JENA-1197 return exit code 1 when there is a problem valida...

Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:

    https://github.com/apache/jena/pull/188
  
    I'm a little mistrustful of a boolean return value there because of an problem @osma dealt with recently wherein he wanted a parse to sort-of succeed but drop bad tuples. Maybe we could return there a marker  `interface ParseCompletion {}` (better name needed) and an out-of-the-box `enum StandardCompletion implements ParseCompletion {SUCCEEDED, FAILED}`? I know that's more complex, but not hugely so and it would bake in some room for expansion or extension.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #188: JENA-1197 return exit code 1 when there is a problem valida...

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:

    https://github.com/apache/jena/pull/188
  
    That looks OK for one file.  What to do about multiple files on the command line is trickier - that is, there are reasonable use cases for different behaviors.
    
    The call stack is (deepest first):
    
    {noformat}
    parseRIOT(String, String, TypedInputStream) : void - riotcmd.CmdLangParse
    parseFile(String, String, TypedInputStream) : void - riotcmd.CmdLangParse
    parseFile(String) : void - riotcmd.CmdLangParse
    exec$() : void - riotcmd.CmdLangParse
    exec() : void - riotcmd.CmdLangParse
    {noformat}
    
    where the multiple file loop is in `exec$()`. I wonder if the "parse*" ought to return true/false (or throw a custom exception) as to whether they succeeded or not and the decision of whether and when to exit is made in `exec$` (just before the `postParse` step). Then an attempt to parse all the files happens and then `exit(1)` if any are broken.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #188: JENA-1197 return exit code 1 when there is a problem valida...

Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the issue:

    https://github.com/apache/jena/pull/188
  
    >Suggestion: we merge this as is, write a proposal on JENA-1197 that each call of a single file parse returns an object describing the outcome and exec$() aggregates the results into a final outcome, including setting the exit code.
    
    Yup, agree. Here's the output when running a command with one valid file, followed by invalid files.
    
    ```
    $ riot --validate /tmp/valid.ttl /tmp/invalid1.ttl /tmp/invalid2.ttl
    
    INFO  File: /tmp/valid.ttl
    INFO  File: /tmp/invalid1.ttl
    ERROR [line: 1, col: 4 ] Unrecognized: FDS
    ```
    
    The valid file is a cheeses ttl file found in Jena sources. The invalid file has some random data.
    
    ```
    cat /tmp/invalid.ttl 
    fdsfd
    ```
    
    Maybe we could merge and fix JENA-1197, then file a new issue to accommodate the more complete changes suggested in this pull request.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #188: JENA-1197 return exit code 1 when there is a problem valida...

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:

    https://github.com/apache/jena/pull/188
  
    The parser has already thrown RiotException by the time this PR comes into play. boolean is to note there is "exit(1)" and not "exit(1)" states.   Boolean, enums or exception are all 2-state returns for that condition; exception and message output or not. This is all within the single class of the command itself
    
    Dropping bad triples needs parser changes. It is a much wider change.
    
    Indeed, there is an argument not to design to a future feature until that feature is fully described!



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #188: JENA-1197 return exit code 1 when there is a problem valida...

Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:

    https://github.com/apache/jena/pull/188
  
    Suggestion: we merge this as is, write a proposal on JENA-1197 that each call of a single file parse returns an object describing the outcome and `exec$()` aggregates the results into a final outcome, including setting the exit code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #188: JENA-1197 return exit code 1 when there is a problem valida...

Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:

    https://github.com/apache/jena/pull/188
  
    Well, the `enum` is just a convenience. It's the `interface` that matters, and the new condition is that parsing completed but tuples were dropped. `exit(n)`? But I admit that I don't understand all of what @osma would want anyway.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---