Add command for opening a shell for Thrift services #161

bradengroom · 2018-02-20T21:29:10Z

This adds a tshell command which will provide a way for Thrift services to open an interactive shell similar to pshell:

$ tshell example.ini
Baseplate Interactive Shell
Python 2.7.6 (default, Nov 23 2017, 15:49:48) 
[GCC 4.8.4]

Available Objects:

  app          This project's app instance
  context      The context for this shell instance's span
>>>

Services may expose additional variables to the shell environment by providing a setup function in their .ini file.

config file:

...
[tshell]
setup = my_service:tshell_setup

setup function:

def tshell_setup(env, env_banner):
    from my_service import models
    env['models'] = models
    env_banner['models'] = 'Models module'

$ tshell example.ini
Baseplate Interactive Shell
Python 2.7.6 (default, Nov 23 2017, 15:49:48) 
[GCC 4.8.4]

Available Objects:

  app          This project's app instance
  context      The context for this shell instance's span
  models       Models module
>>>

bradengroom · 2018-02-20T21:43:30Z

baseplate/server/__init__.py

+
+ args = parser.parse_args(sys.argv[1:])
+ config = read_config(args.config_file, server_name=None, app_name=args.app_name)
+ configure_logging(config, args.debug)


Should I delete this line?

We probably don't want fileConfig taking over, so it might be best to just manually set up a logging.basicConfig(level=logging.INFO) with a simple formatter or something? Not sure how much logging we'd want to see while trying to interact with the REPL.

bradengroom · 2018-02-20T21:44:26Z

setup.py

@@ -89,6 +89,7 @@ def run(self):
 scripts=[
 "bin/baseplate-serve{:d}".format(sys.version_info.major),
 "bin/baseplate-script{:d}".format(sys.version_info.major),
+ "bin/tshell",


Do I need a tshell2 and tshell3 similar to the serve and script commands?

Yeah, I think that makes most sense for consistency.

Should this be baseplate- prefixed? It's almost a generic thrift shell but it does tie into baseplate's span stuff (as it should) so it's definitely custom.

Should this be baseplate- prefixed?

Sounds reasonable. It also doesn't have to be called tshell if you have some other name that you would prefer.

tshell seems pretty nice to be consistent with pshell

bradengroom · 2018-02-20T21:48:40Z

👓 @bsimpson63

spladug

This is looking great and will be super useful. Thank you!

I think it might be nice to add some documentation of this (and pshell tbh). That'd mean adding some new .rst files to docs/ (see CONTRIBUTING.md for how to add new stuff and test it out).

spladug · 2018-02-20T22:10:41Z

baseplate/server/__init__.py

@@ -66,12 +67,16 @@ def read_config(config_file, server_name, app_name):
 if server_name else None)
 app_config = dict(parser.items("app:" + app_name))
 has_logging_config = parser.has_section("loggers")
+ tshell_config = dict()


None might be a little more obvious here.

spladug · 2018-02-20T22:11:38Z

baseplate/server/__init__.py

+
+ args = parser.parse_args(sys.argv[1:])
+ config = read_config(args.config_file, server_name=None, app_name=args.app_name)
+ configure_logging(config, args.debug)


We probably don't want fileConfig taking over, so it might be best to just manually set up a logging.basicConfig(level=logging.INFO) with a simple formatter or something? Not sure how much logging we'd want to see while trying to interact with the REPL.

spladug · 2018-02-20T22:25:16Z

baseplate/server/__init__.py

+ # try to use IPython if possible
+ try:
+ try:
+ # 1.0 <= ipython


Would you be OK dropping the old IPython support?

IPython 1.0 was released on August 8, 2013 (http://ipython.org/news.html)

In general we should probably be writing the minimal version of what we need here rather than just copying pylons; they have many more cases (and much longer history) to deal with.

Sure thing. Yeah, this was copied from r2 which was copied from pylons.

spladug · 2018-02-20T22:25:48Z

baseplate/server/__init__.py

+ shell(local_ns=env, global_ns={})
+ except ImportError:
+ import code
+ py_prefix = sys.platform.startswith('java') and 'J' or 'P'


haha let's drop this too. we're never gonna run in Jython.

spladug · 2018-02-20T22:28:23Z

setup.py

@@ -89,6 +89,7 @@ def run(self):
 scripts=[
 "bin/baseplate-serve{:d}".format(sys.version_info.major),
 "bin/baseplate-script{:d}".format(sys.version_info.major),
+ "bin/tshell",


Yeah, I think that makes most sense for consistency.

Should this be baseplate- prefixed? It's almost a generic thrift shell but it does tie into baseplate's span stuff (as it should) so it's definitely custom.

bradengroom · 2018-02-21T18:34:00Z

💇

spladug

Just pulled this down and played with it and it's great! This is gonna be super useful and make a lot of people happy, thank you!

One request: can you make it clearer in the argparse banner and the sphinx docs that tshell is just for thrift apps?

bradengroom · 2018-02-21T18:51:47Z

One request: can you make it clearer in the argparse banner and the sphinx docs that tshell is just for thrift apps?

8dabc50

Add command for opening a shell for Thrift services

5cdaac3

bradengroom mentioned this pull request Feb 20, 2018

Add baseplate-shell command #151

Closed

bradengroom added 2 commits February 20, 2018 13:33

fixup! Add command for opening a shell for Thrift services

8a92498

fixup! Add command for opening a shell for Thrift services

7c824ed

bradengroom commented Feb 20, 2018

View reviewed changes

spladug self-requested a review February 20, 2018 22:03

spladug suggested changes Feb 20, 2018

View reviewed changes

bradengroom added 2 commits February 21, 2018 10:11

fixup! Add command for opening a shell for Thrift services

76a3fd5

fixup! Add command for opening a shell for Thrift services

e75b3f2

spladug approved these changes Feb 21, 2018

View reviewed changes

fixup! Add command for opening a shell for Thrift services

8dabc50

bradengroom merged commit 6541842 into reddit:master Feb 21, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add command for opening a shell for Thrift services #161

Add command for opening a shell for Thrift services #161

bradengroom commented Feb 20, 2018 •

edited

Loading

bradengroom Feb 20, 2018 •

edited

Loading

spladug Feb 20, 2018

bradengroom Feb 20, 2018

spladug Feb 20, 2018

bradengroom Feb 21, 2018

spladug Feb 21, 2018

bradengroom commented Feb 20, 2018

spladug left a comment

spladug Feb 20, 2018

spladug Feb 20, 2018

spladug Feb 20, 2018

bradengroom Feb 21, 2018

spladug Feb 20, 2018

spladug Feb 20, 2018

bradengroom commented Feb 21, 2018

spladug left a comment

bradengroom commented Feb 21, 2018

Add command for opening a shell for Thrift services #161

Add command for opening a shell for Thrift services #161

Conversation

bradengroom commented Feb 20, 2018 • edited Loading

bradengroom Feb 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradengroom commented Feb 20, 2018

spladug left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradengroom commented Feb 21, 2018

spladug left a comment

Choose a reason for hiding this comment

bradengroom commented Feb 21, 2018

bradengroom commented Feb 20, 2018 •

edited

Loading

bradengroom Feb 20, 2018 •

edited

Loading