This repository was archived by the owner on Jul 22, 2026. It is now read-only.
remove the event loop arguments - #560
Merged
jujubot merged 17 commits intoOct 20, 2021
Merged
jujubot merged 17 commits into
jujubot merged 17 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Author
Sorry, something went wrong.
SimonRichardson
suggested changes
Oct 9, 2021
SimonRichardson
left a comment
Copy link
Copy Markdown
Member
There was a problem hiding this comment.
I think this 100% what we need, but I want to target 3.0 with this, so we don't break 3.5.
Sorry, something went wrong.
don't need to thread the loop object all around since asyncio can create tasks without the loop argument this solves the intermittent issue with calling asyncio.get_event_loop in a worker thread (which errors), as well as gets rid of deprecation warnings for the loop argument. kept all the modifications in a single commit to preserve the code integrity (it's non-trivial to have the tests pass without all the modifications in place)
cderici
force-pushed
the
no-loop-argument
branch
from
8016d74 to
e5b1993
Compare
October 12, 2021 17:09
... that ensures we have all the right bindings for the functions we need from asyncio. Reason for this layer is to accommodate the frequent functional changes, additions and deprecations in asyncio across the different Python versions.
"Manual instantiation of Tasks is discouraged." https://docs.python.org/3/library/asyncio-task.html?highlight=task#asyncio.Task
Copy link
Copy Markdown
Contributor
Author
|
!!build!! |
Sorry, something went wrong.
* use wait_for_idle(status="active") instead of custom block_until-s * model_2 used to wait for "executing" after deploying from file, but if the model intermittently goes into "idle" then this becomes block forever and timeout, now we wait for active
Closed
Copy link
Copy Markdown
Member
|
!!build!! The python3.5 is failing from a setup issue, let's see if another build can get past that. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Interestingly, the Python C header is missing 14:12:37 c/_cffi_backend.c:2:10: fatal error: Python.h: No such file or directory
14:12:37 #include <Python.h>
14:12:37 ^~~~~~~~~~```
|
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
!!build!! |
Sorry, something went wrong.
3 similar comments
Copy link
Copy Markdown
Member
|
!!build!! |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
!!build!! |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
!!build!! |
Sorry, something went wrong.
SimonRichardson
approved these changes
Oct 20, 2021
SimonRichardson
left a comment
Copy link
Copy Markdown
Member
There was a problem hiding this comment.
Looks good to me, the issue around cffi failing to build isn't related to this PR. Looks like an upstream issue.
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
$$merge$$ |
Sorry, something went wrong.
Copy link
Copy Markdown
Yes, cffi 1.15.0 dropped py35 so the last version that works with py35 is cffi 1.14.6. |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
@cderici we'll have to see if we can pin cffi. @ajkavanagh ace, thanks for the info! |
Sorry, something went wrong.
jujubot
added a commit
that referenced
this pull request
Oct 26, 2021
#562 ### Description This PR adds `debug-log` functionality into pylibjuju, by adding a separate Connection object into the Connector of a Model. User runs `await model.debug_log()` with a writable target (defaults to `stdout`), we create a new connection with `/log` and start the receiving tasks. As the user continues to interact with the model, the debug log receiver tasks write the logs onto whatever target is provided. Fixes #549 ### QA Steps ``` python examples/debug-log.py ``` ### Notes & Discussion There are two things need to be done before this is ready to merge: - [x] All the tasks need to be flushed before ending the program (that's why we're getting `ERROR:asyncio:Future exception was never retrieved` errors at the end when we run the example above). This is also closely related to #560 , which I'd prefer to complete and land before I fix this PR. - [x] The filtering parameters of the debug-log (e.g. `no-tail`, `exclude-module`, `level`, `lines`, etc.) need to be passed through.
jujubot
added a commit
that referenced
this pull request
Dec 4, 2021
#599 2.9.5 ^^^^^ Friday December 3 2021 * remove the event loop arguments by @cderici in #560 * add debug-log by @cderici in #562 * Model status by @juanmanuel-tirado in #563 * Pin cffi version to 1.14.6 for Python 3.5 by @cderici in #570 * Wait for applications to terminate on model reset by @balbirthomas in #572 * Babysitting python3.5 by @cderici in #571 * Deploy charmhub bundles by @cderici in #569 * Facade schemas for 2.9.17 by @SimonRichardson in #579 * Bundles with overlays by @cderici in #566 * Consistently getting a unit's public address by @cderici in #573 * [JUJU-158] Add python3.9 to setup.py by @cderici in #585 * [JUJU-157] Add note for removing services by @cderici in #583 * Added boolean entries to normalize values. by @juanmanuel-tirado in #582 * [JUJU-138] Streamlining asyncio tasks/events by @cderici in #580 * [JUJU-234] Fix for small bug in task handling by @cderici in #589 * Ensure all watchers validate for the Id by @SimonRichardson in #592 * [JUJU-276] Facade schemas for 2.9.19 by @cderici in #594 * [JUJU-238] Small bug fix for old ClientFacade support by @cderici in #593 * [JUJU-239] Debug-log parameters by @cderici in #595 * [JUJU-213] Local type `file` resource support by @cderici in #590 * [JUJU-289] Use provided series in deploy if supported by @jack-w-shaw in #596 * [JUJU-292] Update the charms in the tests to use Charmhub by @cderici in #597 * Legacy "services" for describing "applications" within bundles are no longer supported. "applications" can be used as a direct replacement for "services" in bundles.yaml. * The websocket (ws) in a Connection object became a read-only property. [JUJU-293] [JUJU-158]: https://warthogs.atlassian.net/browse/JUJU-158?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Reviewers
1 more reviewer5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Don't need to thread the loop object all around since
asyncio can create tasks without the loop argument.
This solves the intermittent issue with calling asyncio.get_event_loop
in a worker thread (which errors), as well as gets rid of deprecation
warnings for the loop argument. Furthermore, the get_event_loop
is getting deprecated in python3.10 anyways.
Kept all the modifications in a single commit to preserve the code
integrity (it's non-trivial to have the tests pass without all the
modifications in place).
QA Steps
The tests should pass.
Notes & Discussion