JSON wrapper which inputs to a dictionary and outputs a JSON object

Review Request #11521 — Created March 20, 2021 and submitted

ryankang
RBTools
master
11401
11572, 11541, 11588, 11560, 11589, 11590, 11591, 11573
rbtools, students

JSON wrapper class which inputs to a dictionary and outputs a JSON object. Class
is defined in rbtools/commands/__init__.py Command class. New command --json
added to global options, which can be added to any command. When --json is active
run_from_argv will print the json object using Python's json library.

For a full list of proposed JSON objects for each command see:
https://www.notion.so/reviewboard/a8cd6ee32806415fb7bba484d630863e?v=25fb1c9182064421ab17e55606da765b

Passed all tests in Command. Added new tests in test_main.py for initializing
wrapper, adding key value pairs to dictionary, getting correct JSON string from
dictionary, and printing to output stream with correct JSON string.

Summary
merged in Output Wrapper Object from r#11401
added new method to print JSON to stream object and tests to ensure it works properly
added new global command --json
added conditional at the end of command cycle to print JSON is enabled
added method to append to classes and tests to ensure items are appended correctly
added new method to add errors to existing errors key or create a new one
Fixed issues with extra white spaces in files from Review Bot
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput
Fixed minor formatting issues in test_main.py
Updated JSONOutput docstrings to format with guidelines
Updated docstrings in JSONOutputTests in test_main.py to comply with guidelines
Checking if there are any errors in json to decide status
Fixed trailing white space in test_main.test_hson_wrapper_append
Fixed blank lines with spaces in __init__ Command.run_from_argv
Added any CommandError caught to errors key
Fixed bug where test_main.py was imported JSONWrapper instead of JSONOutput
Disabling standard output and error when --json option is enabled
Casting errors as string before adding to JSON object
Description From Last Updated

W293 blank line contains whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E303 too many blank lines (2)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

This class and all the methods inside need docstrings. Perhaps instead of "Wrapper" (it's not really wrapping anything), just call ...

daviddavid

W293 blank line contains whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

Let's make the help be something like "Output results as JSON data instead of text"

daviddavid

W293 blank line contains whitespace

reviewbotreviewbot

This comment isn't necessary--the code is pretty self explanatory.

daviddavid

Looks like this still needs to be done. But initialize_scm_tool isn't the right place to do it.

daviddavid

Let's do import kgb and inherit from kgb.SpyAgency

daviddavid

These two lines can be combined: from rbtools.commands import JSONWrapper, main as rbt_main

daviddavid

It might be worth defining a setUp method that sets self.json = JSONWrapper(sys.stdout). That way you don't have to do ...

daviddavid

E303 too many blank lines (2)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

E127 continuation line over-indented for visual indent

reviewbotreviewbot

E501 line too long (83 > 79 characters)

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

I think it might be better to do self.json.add_error(str(e)) because most of the errors raised (ex. CommandError, ApiError) are not ...

amohapatraamohapatra
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

ryankang
Review request changed

Commits:

Summary
-
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
-
added new method to print JSON to stream object and tests to ensure it works properly
-
added new global command --json
-
added conditional at the end of command cycle to print JSON is enabled
+
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
+
added new method to print JSON to stream object and tests to ensure it works properly
+
added new global command --json
+
added conditional at the end of command cycle to print JSON is enabled
+
added method to append to classes and tests to ensure items are appended correctly
+
added new method to add errors to existing errors key or create a new one

Diff:

Revision 2 (+270 -12)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
  1. 
      
  2. rbtools/commands/__init__.py (Diff revision 2)
     
     

    This class and all the methods inside need docstrings.

    Perhaps instead of "Wrapper" (it's not really wrapping anything), just call it something like "JSONOutput"?

  3. rbtools/commands/__init__.py (Diff revision 2)
     
     

    Let's make the help be something like "Output results as JSON data instead of text"

  4. rbtools/commands/__init__.py (Diff revision 2)
     
     

    This comment isn't necessary--the code is pretty self explanatory.

  5. rbtools/commands/__init__.py (Diff revision 2)
     
     
     
     
     

    Looks like this still needs to be done. But initialize_scm_tool isn't the right place to do it.

    1. Would you recommend putting it at the start of run_from_argv instead?

  6. rbtools/commands/tests/test_main.py (Diff revision 2)
     
     

    Let's do import kgb and inherit from kgb.SpyAgency

  7. rbtools/commands/tests/test_main.py (Diff revision 2)
     
     
     

    These two lines can be combined:

    from rbtools.commands import JSONWrapper, main as rbt_main
    
  8. rbtools/commands/tests/test_main.py (Diff revision 2)
     
     

    It might be worth defining a setUp method that sets self.json = JSONWrapper(sys.stdout). That way you don't have to do it individually in each test.

  9. 
      
ryankang
ryankang
Review request changed

Commits:

Summary
-
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
-
added new method to print JSON to stream object and tests to ensure it works properly
-
added new global command --json
-
added conditional at the end of command cycle to print JSON is enabled
-
added method to append to classes and tests to ensure items are appended correctly
-
added new method to add errors to existing errors key or create a new one
-
Fixed issues with extra white spaces in files from Review Bot
+
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
+
added new method to print JSON to stream object and tests to ensure it works properly
+
added new global command --json
+
added conditional at the end of command cycle to print JSON is enabled
+
added method to append to classes and tests to ensure items are appended correctly
+
added new method to add errors to existing errors key or create a new one
+
Fixed issues with extra white spaces in files from Review Bot
+
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput

Diff:

Revision 4 (+363 -65)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
ryankang
Review request changed

Commits:

Summary
-
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
-
added new method to print JSON to stream object and tests to ensure it works properly
-
added new global command --json
-
added conditional at the end of command cycle to print JSON is enabled
-
added method to append to classes and tests to ensure items are appended correctly
-
added new method to add errors to existing errors key or create a new one
-
Fixed issues with extra white spaces in files from Review Bot
-
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput
-
Fixed minor formatting issues in test_main.py
+
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
+
added new method to print JSON to stream object and tests to ensure it works properly
+
added new global command --json
+
added conditional at the end of command cycle to print JSON is enabled
+
added method to append to classes and tests to ensure items are appended correctly
+
added new method to add errors to existing errors key or create a new one
+
Fixed issues with extra white spaces in files from Review Bot
+
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput
+
Fixed minor formatting issues in test_main.py
+
Updated JSONOutput docstrings to format with guidelines

Diff:

Revision 6 (+441 -79)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
Review request changed

Commits:

Summary
-
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
-
added new method to print JSON to stream object and tests to ensure it works properly
-
added new global command --json
-
added conditional at the end of command cycle to print JSON is enabled
-
added method to append to classes and tests to ensure items are appended correctly
-
added new method to add errors to existing errors key or create a new one
-
Fixed issues with extra white spaces in files from Review Bot
-
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput
-
Fixed minor formatting issues in test_main.py
-
Updated JSONOutput docstrings to format with guidelines
+
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
+
added new method to print JSON to stream object and tests to ensure it works properly
+
added new global command --json
+
added conditional at the end of command cycle to print JSON is enabled
+
added method to append to classes and tests to ensure items are appended correctly
+
added new method to add errors to existing errors key or create a new one
+
Fixed issues with extra white spaces in files from Review Bot
+
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput
+
Fixed minor formatting issues in test_main.py
+
Updated JSONOutput docstrings to format with guidelines
+
Updated docstrings in JSONOutputTests in test_main.py to comply with guidelines

Diff:

Revision 7 (+456 -98)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
Review request changed

Commits:

Summary
-
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
-
added new method to print JSON to stream object and tests to ensure it works properly
-
added new global command --json
-
added conditional at the end of command cycle to print JSON is enabled
-
added method to append to classes and tests to ensure items are appended correctly
-
added new method to add errors to existing errors key or create a new one
-
Fixed issues with extra white spaces in files from Review Bot
-
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput
-
Fixed minor formatting issues in test_main.py
-
Updated JSONOutput docstrings to format with guidelines
-
Updated docstrings in JSONOutputTests in test_main.py to comply with guidelines
+
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
+
added new method to print JSON to stream object and tests to ensure it works properly
+
added new global command --json
+
added conditional at the end of command cycle to print JSON is enabled
+
added method to append to classes and tests to ensure items are appended correctly
+
added new method to add errors to existing errors key or create a new one
+
Fixed issues with extra white spaces in files from Review Bot
+
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput
+
Fixed minor formatting issues in test_main.py
+
Updated JSONOutput docstrings to format with guidelines
+
Updated docstrings in JSONOutputTests in test_main.py to comply with guidelines
+
Checking if there are any errors in json to decide status

Diff:

Revision 8 (+466 -98)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
Review request changed

Commits:

Summary
-
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
-
added new method to print JSON to stream object and tests to ensure it works properly
-
added new global command --json
-
added conditional at the end of command cycle to print JSON is enabled
-
added method to append to classes and tests to ensure items are appended correctly
-
added new method to add errors to existing errors key or create a new one
-
Fixed issues with extra white spaces in files from Review Bot
-
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput
-
Fixed minor formatting issues in test_main.py
-
Updated JSONOutput docstrings to format with guidelines
-
Updated docstrings in JSONOutputTests in test_main.py to comply with guidelines
-
Checking if there are any errors in json to decide status
+
added JSON wrapper class to main.py which inputs to a dictionary and outputs a JSON object. Added tests for JSON wrapper functionality in test_main.py
+
added new method to print JSON to stream object and tests to ensure it works properly
+
added new global command --json
+
added conditional at the end of command cycle to print JSON is enabled
+
added method to append to classes and tests to ensure items are appended correctly
+
added new method to add errors to existing errors key or create a new one
+
Fixed issues with extra white spaces in files from Review Bot
+
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput
+
Fixed minor formatting issues in test_main.py
+
Updated JSONOutput docstrings to format with guidelines
+
Updated docstrings in JSONOutputTests in test_main.py to comply with guidelines
+
Checking if there are any errors in json to decide status
+
Fixed trailing white space in test_main.test_hson_wrapper_append

Diff:

Revision 9 (+467 -99)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
ryankang
amohapatra
  1. 
      
  2. rbtools/commands/__init__.py (Diff revision 11)
     
     

    I think it might be better to do self.json.add_error(str(e)) because most of the errors raised (ex. CommandError, ApiError) are not JSON serializable.

  3. 
      
ryankang
ryankang
ryankang
ryankang
ryankang
ryankang
david
Review request changed

Status: Closed (submitted)

Loading...