New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't check call tty on macs #5760

Merged
merged 1 commit into from Jan 29, 2019

Conversation

Projects
4 participants
@dschep
Copy link
Member

dschep commented Jan 28, 2019

What did you implement:

In #5354/#5355 a call to tty was called to ensure it was available to prevent issues in CI systems. However, when I merged it, I didn't have a mac and saw no issue with this. When I joined the team at serverless, I got a mac so I could better test the UX that our many mac users experience. I've found that this tty call doesn't work on macs, so I'm disabling this check on macOs since there shouldn't be odd environments like CI to consider on a mac;

How did you implement it:

skip call to tty when sys.platform == 'darwin'

How can we verify it:

On a mac (no changes on other OSes)

sls create -t aws-python3
echo -e 'def hello(event, context):\n    import pdb;pdb.set_trace()' > handler.py
sls invoke local -f hello
--Return--

> /Users/dschep/code/tmp/handler.py(2)hello()->None
-> import pdb;pdb.set_trace()
(Pdb)

on master it raises bdb.BdbQuit

Todos:

  • n/a Write tests
  • n/a Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@dschep dschep requested review from pmuens and eahefnawy Jan 28, 2019

@AndrewFarley

This comment has been minimized.

Copy link
Contributor

AndrewFarley commented Jan 28, 2019

LGTM, confirmed functional on Mac OS 10.14.1 Mojave, thanks!

@pmuens pmuens added the pr/in-review label Jan 29, 2019

@pmuens

pmuens approved these changes Jan 29, 2019

Copy link
Member

pmuens left a comment

LGTM :shipit:

Just tested it on my mac 👍 Thanks for adding steps to test this!

@pmuens pmuens self-assigned this Jan 29, 2019

@pmuens pmuens added this to In progress in Serverless via automation Jan 29, 2019

@pmuens pmuens merged commit 5da5e72 into master Jan 29, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

Serverless automation moved this from In progress to Done Jan 29, 2019

@pmuens pmuens deleted the pdb-mac branch Jan 29, 2019

@pmuens pmuens added this to the 1.36.4 milestone Feb 5, 2019

@shortjared shortjared added the bug label Feb 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment