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

unnecessary warnings #110

Open
hartytp opened this issue May 8, 2019 · 6 comments
Open

unnecessary warnings #110

hartytp opened this issue May 8, 2019 · 6 comments

Comments

@hartytp
Copy link

hartytp commented May 8, 2019

Currently, a warning is generated whenever an exception is raised in a future on Windows

self._logger.warning('Event callback failed', exc_info=sys.exc_info())
Since many (most?) in exceptions aren't really exceptional at all, this can lead to a lot of unwanted noise in the logs, obscuring more important warnings.

Can we downgrade this to a debug message?

e.g. consider something like

while True:
    # try to connect to server
    while True:
        try:
            self.reader, self.writer = \
                await asyncio.open_connection(host, port)
                break
        except ConnetionError:
            # server isn't ready yet, retry in 10s
            await asyncio.sleep(10)
    
    while True:
        try:
            line = await self.reader.readline()
            # do something
        except ConnectionError:
            # server down, try to reconnect
            break

All exceptions here are caught and handled, so there is nothing that warrants quamash generating a warning in the log (it should be up to the user to log server disconnects if they feel it's worth it).

@harvimt
Copy link
Owner

harvimt commented May 9, 2019

when submitting a patch like this, it's customary to submit it as a PR this... is kind of the point of github.

Also the thing actually blocking quamash development at this point, is the fact the test suite doesn't run.

Let me see if I can get the test suite working, and probably also drop support for Python 3.4 and 3.5... and maybe even 3.6 and PyQt4 and PySide.

@hartytp
Copy link
Author

hartytp commented May 9, 2019

done

@harvimt
Copy link
Owner

harvimt commented May 9, 2019

I fixed the tests in the "tests-2019" branch #111 so probably rebase off that branch

@hartytp
Copy link
Author

hartytp commented May 9, 2019

Will do. Should I wait until you merge #111 first?

@harvimt
Copy link
Owner

harvimt commented May 9, 2019 via email

@hartytp
Copy link
Author

hartytp commented May 10, 2019

Okay, I'll do that. FWIW, I would have probably done it the other way: merge a series of small commits into master, and then update the readme + tag at the end.

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

No branches or pull requests

2 participants