Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@cgranade
Copy link
Contributor

This PR delays starting services required by IQSharpEngine until the Engine.Start method is called, so as to not block proper handling of shell messages for reporting startup status to the client. This PR also follows the advice of @cesarzc, and initializes Bond protocol serialization and deserialization in the engine, so that the Q# compilation loader is ready earlier.

(Marked as draft until a dependency on microsoft/jupyter-core#71 can be taken.)

@cgranade cgranade requested review from cesarzc and vxfield January 14, 2021 02:47
@cgranade cgranade marked this pull request as draft January 14, 2021 02:47
@cgranade
Copy link
Contributor Author

This turned into a lot more of a nullability improvement PR than I intended, as adapting a test wound up involving chasing down everywhere that uninitialized services could result in null reference exceptions. Apologies for the noisy PR as a result of that.

@cgranade cgranade marked this pull request as ready for review January 14, 2021 23:22
@cgranade
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

guenp
guenp previously approved these changes Jan 14, 2021
@cgranade
Copy link
Contributor Author

It looks like something else is going wrong with CI, such that tests are hanging without an explicit failure. I'm unable to reproduce locally yet; it seems like this only happens in CI, but I will keep investigating.

@cgranade
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@cgranade
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@cgranade
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@cgranade
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@cgranade
Copy link
Contributor Author

In investigating recent CI failures, it appears that microsoft/jupyter-core#71 may have introduced a subtle Jupyter messaging protocol bug. With 566b5b9, I'm currently testing a fix in jupyter-core; if it works, will open a PR for that fix shortly and will push a new release.

@cgranade
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@cgranade
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@cgranade
Copy link
Contributor Author

It looks like this resolved the issue, so I will open a PR on jupyter-core, and then will update this PR once that is resolved.

guenp
guenp previously approved these changes Jan 15, 2021
@cgranade
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@cgranade
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@cgranade cgranade merged commit d307953 into main Jan 19, 2021
@cgranade cgranade deleted the cgranade/services-engine-start branch January 19, 2021 18:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iqsharp jupyter kernel connection fails due to tornado timeout error

4 participants