Skip to content
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

Add support for Async operations on Worker #38

Merged
merged 20 commits into from
Jun 14, 2020
Merged

Conversation

ocalvo
Copy link
Contributor

@ocalvo ocalvo commented Jun 10, 2020

No description provided.

@codecov
Copy link

codecov bot commented Jun 10, 2020

Codecov Report

Merging #38 into master will increase coverage by 2.37%.
The diff coverage is 62.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
+ Coverage   38.46%   40.84%   +2.37%     
==========================================
  Files          46       49       +3     
  Lines        1760     1944     +184     
  Branches      219      230      +11     
==========================================
+ Hits          677      794     +117     
- Misses       1061     1123      +62     
- Partials       22       27       +5     
Impacted Files Coverage Δ
examples/async.py 0.00% <0.00%> (ø)
gammu/__init__.py 100.00% <ø> (ø)
gammu/asyncworker.py 84.69% <84.69%> (ø)
test/test_asyncworker.py 97.14% <97.14%> (ø)
gammu/worker.py 88.18% <0.00%> (+2.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd59c2d...2f1c9e5. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Jun 10, 2020

This pull request introduces 1 alert when merging 5d28353 into 86a497c - view on LGTM.com

new alerts:

  • 1 for Syntax error

@nijel
Copy link
Member

nijel commented Jun 10, 2020

Can you please add some tests for this code?

@ocalvo
Copy link
Contributor Author

ocalvo commented Jun 10, 2020

Can you please add some tests for this code?

Certainly. Thanks for the quick reply.

@lgtm-com
Copy link

lgtm-com bot commented Jun 12, 2020

This pull request introduces 1 alert when merging ac8c0ff into 86a497c - view on LGTM.com

new alerts:

  • 1 for Syntax error

@lgtm-com
Copy link

lgtm-com bot commented Jun 12, 2020

This pull request introduces 1 alert when merging 78138f1 into 86a497c - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

@ocalvo
Copy link
Contributor Author

ocalvo commented Jun 13, 2020

Can you please add some tests for this code?

Certainly. Thanks for the quick reply.

How do you run tests locally?

@ocalvo
Copy link
Contributor Author

ocalvo commented Jun 13, 2020

Can you please add some tests for this code?

Certainly. Thanks for the quick reply.

How do you run tests locally?

I am sorry, but looks like I wont be able to add tests, the test suite is running with python 2 and the async keyword is not supported:

Traceback (most recent call last):
  File "setup.py", line 240, in <module>
    ext_modules=[get_module()]
  File "C:\Python27\lib\site-packages\setuptools\__init__.py", line 145, in setup
    return distutils.core.setup(**attrs)
  File "C:\Python27\lib\distutils\core.py", line 151, in setup
    dist.run_commands()
  File "C:\Python27\lib\distutils\dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "C:\Python27\lib\distutils\dist.py", line 972, in run_command
    cmd_obj.run()
  File "C:\Python27\lib\site-packages\setuptools\command\test.py", line 229, in run
    self.run_tests()
  File "C:\Python27\lib\site-packages\setuptools\command\test.py", line 251, in run_tests
    exit=False,
  File "C:\Python27\lib\unittest\main.py", line 94, in __init__
    self.parseArgs(argv)
  File "C:\Python27\lib\unittest\main.py", line 149, in parseArgs
    self.createTests()
  File "C:\Python27\lib\unittest\main.py", line 158, in createTests
    self.module)
  File "C:\Python27\lib\unittest\loader.py", line 130, in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "C:\Python27\lib\unittest\loader.py", line 103, in loadTestsFromName
    return self.loadTestsFromModule(obj)
  File "C:\Python27\lib\site-packages\setuptools\command\test.py", line 55, in loadTestsFromModule
    tests.append(self.loadTestsFromName(submodule))
  File "C:\Python27\lib\unittest\loader.py", line 91, in loadTestsFromName
    module = __import__('.'.join(parts_copy))
  File "c:\projects\python-gammu\test\test_asyncworker.py", line 34
    async def test_worker_async(self):
            ^
SyntaxError: invalid syntax
Command exited with code 1

@nijel
Copy link
Member

nijel commented Jun 13, 2020

It's probably time to drop Python 2 support, feel free to make tests Python 3 only.

@ocalvo
Copy link
Contributor Author

ocalvo commented Jun 13, 2020

It's probably time to drop Python 2 support, feel free to make tests Python 3 only.

Sounds good to me.

@nijel
Copy link
Member

nijel commented Jun 13, 2020

I've just dropped Python 2 support in d181cb4

@ocalvo
Copy link
Contributor Author

ocalvo commented Jun 13, 2020

I've just dropped Python 2 support in d181cb4

Do I need another release for 3.1 or we can ship async with 3.0?

@nijel
Copy link
Member

nijel commented Jun 13, 2020

It will be with 3.0, there is no plan for it yet.


import sys
import pprint
pp = pprint.PrettyPrinter(indent=4)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like forgotten debugging code?

@ocalvo
Copy link
Contributor Author

ocalvo commented Jun 13, 2020

Scrutinizer failed with The code coverage data was not received within the specified time. Please make sure your third-party service is configured to send code coverage data.
Am I missing something?

@nijel nijel merged commit 13927d8 into gammu:master Jun 14, 2020
@nijel
Copy link
Member

nijel commented Jun 14, 2020

Merged, thanks for your contribution!

@ocalvo ocalvo deleted the Async branch June 14, 2020 04:22
@ocalvo
Copy link
Contributor Author

ocalvo commented Jun 14, 2020

Thanks, when do you think you can release 3.0?
We need this to implement incoming SMS notifications in Home Assistant

@nijel
Copy link
Member

nijel commented Jun 14, 2020

I think I can do it now :-)

@ocalvo
Copy link
Contributor Author

ocalvo commented Jun 14, 2020

I think I can do it now :-)

Awesome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants