You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by GitBox <gi...@apache.org> on 2021/06/21 01:12:58 UTC

[GitHub] [avro] kojiromike opened a new pull request #1270: Update Python CLI

kojiromike opened a new pull request #1270:
URL: https://github.com/apache/avro/pull/1270


   Improves the python cli, consisting mainly of scripts/avro and avro.tool, by implementing argparse, and a `__main__.py` module.
   
   - argparse handles arity and type exceptions more fluently at invocation time than can easily be done with sys.argv.
   - argparse handles sub-commands natively, especially in python 3.7 and up.
   - `__main__.py` dispatch (along with setuptools `entry_points.console_scripts` at install time) allows the cli to be invoked with both `python -m avro` and just `avro` without having an out-of-band `scripts/avro` directory.
   - Just in case someone is still using scripts/avro, I've kept that around with a deprecation warning.
   - I added type hints as best I can; however, there are some gray areas with the CLI:
     - avro prefers its fileIO to be bytes, but the standardio are text.
     - The avro script has an `eval` call that we might want to get rid of, some day.
   
   I'm not sure why we have both avro.tool and avro. We may want to converge on a single avro entrypoint.
   
   ### Jira
   
   - [ ] My PR addresses the following [Avro Jira](https://issues.apache.org/jira/browse/AVRO/) issues and references them in the PR title. For example, "AVRO-1234: My Avro PR"
     - https://issues.apache.org/jira/browse/AVRO-XXX
     - In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   
   ### Tests
   
   - [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
   
   ### Commits
   
   - [ ] My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](https://chris.beams.io/posts/git-commit/)":
     1. Subject is separated from body by a blank line
     1. Subject is limited to 50 characters (not including Jira issue reference)
     1. Subject does not end with a period
     1. Subject uses the imperative mood ("add", not "adding")
     1. Body wraps at 72 characters
     1. Body explains "what" and "why", not "how"
   
   ### Documentation
   
   - [ ] In case of new functionality, my PR adds documentation that describes how to use it.
     - All the public functions and the classes in the PR contain Javadoc that explain what it does
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [avro] RyanSkraba commented on a change in pull request #1270: AVRO-3162: Update Python CLI

Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on a change in pull request #1270:
URL: https://github.com/apache/avro/pull/1270#discussion_r660660780



##########
File path: lang/py/avro/schema.py
##########
@@ -1189,27 +1190,22 @@ def make_avsc_object(json_data: object, names: Optional[avro.name.Names] = None,
     raise avro.errors.SchemaParseException(fail_msg)
 
 
-# TODO(hammer): make method for reading from a file?
-
-
-def parse(json_string, validate_enum_symbols=True):
+def parse(json_string: str, validate_enum_symbols: bool = True) -> Schema:
     """Constructs the Schema from the JSON text.
 
     @arg json_string: The json string of the schema to parse
     @arg validate_enum_symbols: If False, will allow enum symbols that are not valid Avro names.
     @return Schema
     """
-    # parse the JSON
     try:
         json_data = json.loads(json_string)
-    except Exception as e:
-        msg = f"Error parsing JSON: {json_string}, error = {e}"
-        new_exception = avro.errors.SchemaParseException(msg)
-        traceback = sys.exc_info()[2]
-        raise new_exception.with_traceback(traceback)
+    except json.decoder.JSONDecodeError as e:
+        raise avro.errors.SchemaParseException(f"Error parsing JSON: {json_string}, error = {e}") from e
+    return make_avsc_object(json_data, Names(), validate_enum_symbols)
 
-    # Initialize the names object
-    names = Names()
 
-    # construct the Avro Schema object
-    return make_avsc_object(json_data, names, validate_enum_symbols)
+def from_path(path: Union[Path, str], validate_enum_symbols: bool = True) -> Schema:

Review comment:
       Nice!




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [avro] kojiromike merged pull request #1270: AVRO-3162: Update Python CLI

Posted by GitBox <gi...@apache.org>.
kojiromike merged pull request #1270:
URL: https://github.com/apache/avro/pull/1270


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [avro] subhashb commented on pull request #1270: AVRO-3162: Update Python CLI

Posted by GitBox <gi...@apache.org>.
subhashb commented on pull request #1270:
URL: https://github.com/apache/avro/pull/1270#issuecomment-876476378


   👌  👍 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [avro] kojiromike commented on pull request #1270: AVRO-3162: Update Python CLI

Posted by GitBox <gi...@apache.org>.
kojiromike commented on pull request #1270:
URL: https://github.com/apache/avro/pull/1270#issuecomment-876511632


   I'm going to take a little more time to consider the issues with TextIO versus BinaryIO, particularly when stdout/err are ttys.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [avro] kojiromike commented on a change in pull request #1270: AVRO-3162: Update Python CLI

Posted by GitBox <gi...@apache.org>.
kojiromike commented on a change in pull request #1270:
URL: https://github.com/apache/avro/pull/1270#discussion_r697919504



##########
File path: lang/py/avro/test/test_script.py
##########
@@ -193,26 +178,29 @@ def tearDown(self):
                 continue
 
     def _run(self, *args, **kw):
-        args = [sys.executable, SCRIPT, "write", "--schema", self.schema_file] + list(args)
+        args = [sys.executable, "-m" "avro", "write", "--schema", self.schema_file] + list(args)

Review comment:
       Hah, this one is funny, because `python -mavro` works just as well as `python -m avro`, but it was a good find by the code scanner anyway.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org