Skip to content

Remove deprecated commands/Commands/FrameChange/PersistentValue#4671

Merged
mergify[bot] merged 13 commits intoQiskit:masterfrom
lcapelluto:remove-deprecated-commands
Jul 17, 2020
Merged

Remove deprecated commands/Commands/FrameChange/PersistentValue#4671
mergify[bot] merged 13 commits intoQiskit:masterfrom
lcapelluto:remove-deprecated-commands

Conversation

@lcapelluto
Copy link
Copy Markdown
Contributor

Summary

Remove deprecated commands! 🙌

Details and comments

@lcapelluto lcapelluto force-pushed the remove-deprecated-commands branch from eeec6d2 to fbb641f Compare July 8, 2020 18:08
Copy link
Copy Markdown
Contributor

@taalexander taalexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a reno this should be good to go.

@lcapelluto
Copy link
Copy Markdown
Contributor Author

I don't know what's going on. Somehow the docs are taking over an hour to build, so the build gets cancelled. Not sure what would cause it to hang like that.

@mtreinish
Copy link
Copy Markdown
Member

I don't know what's going on. Somehow the docs are taking over an hour to build, so the build gets cancelled. Not sure what would cause it to hang like that.

This is something I noticed yesterday (or earlier this week the days have been blending together for me lately), it looks like since we enabled parallel sphinx builds in ci with #4477 we've triggered a bug in jupyter execute (which I still need to reproduce and report upstream). Basically it looks like when a jupyter cell exits with an error/exception the parallel worker stops without reporting it's done to the parent process, so the sphinx build sits waiting for a failed worker process to finish forever. Well until we hit the azure pipeline timeout and the job gets cancelled. I'd suggest trying the docs build locally (probably removing -j auto from the tox.ini so it doesn't sit waiting locally too) to test and find the error, or just relaunch the job and watch the progress before it gets cancelled, it should print out the error.

My guess based on this PR is that there is a jupyter execute somewhere in the docs or release notes that's using the deprecated things being removed here that's crashing the docs builds.

@SooluThomas
Copy link
Copy Markdown
Member

@mtreinish Is using GitHub Actions for docs build a good option here?

@mtreinish
Copy link
Copy Markdown
Member

@mtreinish Is using GitHub Actions for docs build a good option here?

That doesn't really solve the problem here, it would actually make it worse since the timeout is 6 hours instead of 1 hour so we'd be eating up a job for much longer. The parallel docs jobs for terra typically take about 12-13 min which is far from being in danger of hitting a timeout. The only reason we're having an issue here now is because of a bug in the jupyter-sphinx extension with parallel execution. We could turn off parallel execution and docs builds would go back up to 15-20min running serially but we'd avoid the bug where a failure in jupyter-execute results in a job being cancelled because of a timeout. But, we switched to parallel sphinx builds to get a faster feedback time on CI, at 20mins for docs we'd typically be > 30min for full results on an idle pipeline and much longer when there are lots of patches in flight, so I'm not sure if we want to do that.

@lcapelluto lcapelluto force-pushed the remove-deprecated-commands branch from e215de0 to 5db181e Compare July 10, 2020 21:22
@lcapelluto lcapelluto dismissed taalexander’s stale review July 10, 2020 21:32

added a release note!

@lcapelluto lcapelluto added this to the 0.15 milestone Jul 15, 2020
Copy link
Copy Markdown
Contributor

@taalexander taalexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can get passing tests, I believe this is good to go 😄

# new
Instruction(args, channel)

::
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems odd, but maybe I don't know enough about what it is doing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to make two separate literal blocks

@mergify mergify Bot merged commit a9b68a5 into Qiskit:master Jul 17, 2020
drewrisinger added a commit to drewrisinger/qiskit-terra that referenced this pull request Aug 17, 2020
Follow-up to Qiskit#4671.
Removes any remaining "pv"/PersistentValue references in examples/tests.
Leaving entry in qiskit/schemas/qobj_schema.json for validation
purposes.
mergify Bot added a commit that referenced this pull request Aug 19, 2020
Follow-up to #4671.
Removes any remaining "pv"/PersistentValue references in examples/tests.
Leaving entry in qiskit/schemas/qobj_schema.json for validation
purposes.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

4 participants