fix: load compose logs async not await #3377
Merged
Conversation
This file contains 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
benoitf
reviewed
Jul 31, 2023
| await window.logsContainer(container.engineId, container.id, logsCallback); | ||
| // Get the logs for the container asynchronously / all at the same time | ||
| // this will update the terminal as the logs come in. | ||
| window.logsContainer(container.engineId, container.id, logsCallback); |
There was a problem hiding this comment.
I think there is a missing catch here
( I think an eslint rule is still not applied but it would fail)
benoitf
requested changes
Jul 31, 2023
cdrage
force-pushed
the
load-logs-async
branch
2 times, most recently
from
16ae392
to
6f5b0cf
Compare
### What does this PR do? * Loads all the compose containers asynchronously rather than using await. * Adds simple tests for No logs, however, will have to add integration test for loading logs correcctly ### Screenshot/screencast of this PR <!-- Please include a screenshot or a screencast explaining what is doing this PR --> ### What issues does this PR fix or reference? <!-- Please include any related issue from Podman Desktop repository (or from another issue tracker). --> Closes containers#3349 ### How to test this PR? <!-- Please explain steps to reproduce --> 1. Test using Docker Compose example: https://gist.githubusercontent.com/benoitf/a94cff3545cb0992007dd1fc58287816/raw/d0b33311834d84ec1d6830b32a167af80c84926e/docker-compose-example.yaml 2. Compose logs will appear asynchronously now / while podman desktop retrieves the logs, rather than await for each container Signed-off-by: Charlie Drage <charlie@charliedrage.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
fix: load compose logs async not await
What does this PR do?
test for loading logs correcctly
Screenshot/screencast of this PR
Screen.Recording.2023-07-31.at.12.03.03.PM.mov
What issues does this PR fix or reference?
Closes #3349
How to test this PR?
retrieves the logs, rather than await for each container
Signed-off-by: Charlie Drage charlie@charliedrage.com