quamash icon indicating copy to clipboard operation
quamash copied to clipboard

unnecessary warnings

Open hartytp opened this issue 5 years ago • 6 comments

Currently, a warning is generated whenever an exception is raised in a future on Windows https://github.com/harvimt/quamash/blob/e513b30f137415c5e098602fa383e45debab85e7/quamash/_windows.py#L44 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).

hartytp avatar May 08 '19 08:05 hartytp

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.

harvimt avatar May 09 '19 01:05 harvimt

done

hartytp avatar May 09 '19 12:05 hartytp

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

harvimt avatar May 09 '19 18:05 harvimt

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

hartytp avatar May 09 '19 21:05 hartytp

the problem is #111 updates the readme and version number, so I'd prefer to get everything in a 0.7.0 branch, then merge and release all at once.

So if you could rebase off that branch and then set your branch to merge to it instead of master, that's probably best.

Then I can merges tests-2019 when it is ready to release.

On Thu, May 9, 2019 at 2:30 PM hartytp [email protected] wrote:

Will do. Should I wait until you merge #111 https://github.com/harvimt/quamash/pull/111 first?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/harvimt/quamash/issues/110#issuecomment-491076076, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA5CQSL3F3N5JPIK3CPQFTPUSJW3ANCNFSM4HLPPDBQ .

harvimt avatar May 09 '19 23:05 harvimt

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.

hartytp avatar May 10 '19 08:05 hartytp