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

AVRO-2445: Remove StoppableHTTPServer Polyfill #567

Merged

Conversation

kojiromike
Copy link
Contributor

No description provided.

@kojiromike kojiromike force-pushed the AVRO-2445/stoppable-http-server branch from 7a1ca4c to 645d18c Compare June 26, 2019 13:14
@nandorKollar
Copy link
Contributor

@kojiromike It looks like the test failures might be related to the removal of StoppableHTTPServer, because the last log entry is about stopping the service. I tried to reproduce it locally in my environment, but couldn't: for me these interoperability tests passed (Python 2.7.10 is installed on my desktop). Could you please have a look at the failure?

@kojiromike
Copy link
Contributor Author

This passes for me locally as well. I'll have to dig deeper into why it's failing just for TravisCI.

@kojiromike kojiromike force-pushed the AVRO-2445/stoppable-http-server branch from 645d18c to adf9cc1 Compare July 18, 2019 00:54
@kojiromike
Copy link
Contributor Author

Apparently I have to explicitly shutdown the server in a thread. That makes sense, but what doesn't make sense is why the previous implementation did not require it. 😕

@kojiromike
Copy link
Contributor Author

@nandorKollar Fixed now, please re-review.

@nandorKollar
Copy link
Contributor

Thanks @kojiromike. I've little Python knowledge, therefore I don't know why it wasn't required before.

@nandorKollar nandorKollar merged commit efe1181 into apache:master Jul 23, 2019
@kojiromike kojiromike deleted the AVRO-2445/stoppable-http-server branch July 23, 2019 14:40
kojiromike added a commit to kojiromike/avro that referenced this pull request Jan 12, 2020
RyanSkraba pushed a commit to kojiromike/avro that referenced this pull request Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants