SG-17861 Fixes an error that gets thrown when leaving a project on OSX.

[pscadding]

When you leave a project (after fully loading) in SG Desktop on OSX you get the following error in the console (but not in the logs):

Traceback (most recent call last):
  File "/Users/philips1/source_code/tk-desktop/python/tk_desktop/rpc.py", line 285, in run
    [self.server._listener._socket], [], [], self.LISTEN_TIMEOUT
error: (9, 'Bad file descriptor')

It seems it errors if the connection gets closed mid loop, then the select.select call will error if the listen object is closed.
I'm not sure if this is the right way of going about handling this.

[codecov]

Codecov Report

Merging #120 into master will increase coverage by 0.03%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master     #120      +/-   ##
==========================================
+ Coverage   15.24%   15.28%   +0.03%     
==========================================
  Files          68       68              
  Lines        4533     4554      +21     
==========================================
+ Hits          691      696       +5     
- Misses       3842     3858      +16     

Impacted Files	Coverage Δ
python/tk_desktop/rpc.py	88.03% <83.33%> (+0.29%)	⬆️
...k_desktop/desktop_engine_project_implementation.py	0.00% <0.00%> (ø)

---

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 9eec2e0...44cd196. Read the comment docs.

[pscadding]

I looks like there are some other errors thrown at other stages such as if you press back whilst loading a project or if you close desktop whilst loading a project. I'll make those part of this PR so actually hold off on the review for now.

[jfboismenu]

Looks fine, but the Python 3 build is broken.

[jfboismenu]

Ah, I like it! Smart.

[pscadding]

yeah I like it as well but as per our discussion I'll remove it, as we are not going to go and fix the issues it is raising right now in the tests.

It did actually however prove that my fix worked.

[pscadding]

OK I've actually left it in and commented out the raising. How do you feel about that?
Would you prefer me to remove it entirely. I feel by leaving it in there we are drawing attention to the fact that we are not capturing the errors from the server and providing a solution that could be enabled when we are ready to deal with the errors.

another approach I guess would be to remove the class but just leave a TODO with the link to the SO answer that gave me the solution.

[jfboismenu]

Well, if you have inspired the code from something you saw on StackOverflow, you are legally required to include a link. All posts on StackOverflow are done under the Creative Commons license. It requires that any piece of code you reuse or adapt is given attribution by providing a link to the required post.

I also just learned that you need to mention if the code was modified or as is. I've generally been using the following formulations: "This code has been lifted" vs "This code has been inspired by".

[jfboismenu]

I'd rather put a comment in the code itself with a FIXME saying that a exception can be raised from here, so the next person who needs to fix this can do it can find the place to fix right away. What do you think?

[pscadding]

AH OK I didn't know about the inspired by bit, but I did have the link in there.

I have a TODO which I could rename to FIXME, are you saying just have a comment/FIXME with the link and remove the class entirely rather than just commenting out the raise bit in the class?

[jfboismenu]

From my understand the class is not used. Instead of leaving a test class with some magic code in it, let's document the actual class with its shortcoming. Does that make sense?

[pscadding]

I've changed the approach again. Francis was getting a different error than originally reported when testing this. And when I went back to test it I was getting this different error as well, and I couldn't reproduce the originally reported error.
I think it comes down to luck/speed, and depending when the server gets closed, you get a different error.
So I thought probably the best thing to do is catch all errors, and then check the self._is_closed property, however its possible this hasn't yet been set, so I've included a small sleep to give it a chance, and then if it is still not set, raise the error.

What do you think?