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.

Sign up for GitHub

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

Jump to bottom

IPC blocking fix #5400

Merged
merged 8 commits into from
Apr 17, 2023
Merged

IPC blocking fix #5400

merged 8 commits into from
Apr 17, 2023
+86 −25

Conversation

swbsi
Copy link
Contributor

@swbsi swbsi commented Apr 14, 2023

Added tests to ensure we match the electron behavior.
Previously, the backend would block until a request completed, which prevented concurrent scenarios like canceling downloads from working.

swbsi added 2 commits April 14, 2023 15:24
@swbsi
diff

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
39d378d
@swbsi
change
68946f1
@swbsi swbsi requested review from calebmshafer, wgoehrig and a team as code owners April 14, 2023 19:58
pmconne
pmconne approved these changes Apr 14, 2023
View reviewed changes
Copy link
Member

@pmconne pmconne left a comment

Choose a reason for hiding this comment

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

Need to run lint.

@aruniverse
Copy link
Member

@Mergifyio backport release/3.7.x

@mergify
Copy link
Contributor

mergify bot commented Apr 14, 2023 •
edited
Loading

backport release/3.7.x

✅ Backports have been created

@swbsi
Copy link
Contributor Author

swbsi commented Apr 14, 2023

Looks like my onlys were hiding some failing tests. Looking into it…

@pmconne
Copy link
Member

pmconne commented Apr 14, 2023

Looks like my onlys were hiding some failing tests.

Only Linux failed, looks like potentially unrelated flakiness. I requeued to see if they recur.

@swbsi
Copy link
Contributor Author

swbsi commented Apr 14, 2023

Looks like my onlys were hiding some failing tests.

Only Linux failed, looks like potentially unrelated flakiness. I requeued to see if they recur.

Looks like that was it. My local runs also succeed (on windows)...

@pmconne
Copy link
Member

pmconne commented Apr 14, 2023

@wgoehrig @calebmshafer awaiting your approval.

@aruniverse
Copy link
Member

@tcobbs-bentley @toddsouthenbentley plz verify this fixes your mobile issues before the pr is completed

@toddsouthenbentley
Copy link
Contributor

toddsouthenbentley commented Apr 17, 2023 •
edited
Loading

plz verify this fixes your mobile issues before the pr is completed

@aruniverse I think this would need to be back ported to 3.x before we can test on mobile (due to iTwin.js 4.0 not currently running on mobile)

@aruniverse aruniverse merged commit 961c478 into master Apr 17, 2023
@aruniverse aruniverse deleted the ipc-fix branch April 17, 2023 16:24
mergify bot pushed a commit that referenced this pull request Apr 17, 2023
@swbsi @mergify
IPC blocking fix (#5400)
46f590c 
Co-authored-by: Paul Connelly <22944042+pmconne@users.noreply.github.com>
(cherry picked from commit 961c478)
@mergify mergify bot mentioned this pull request Apr 17, 2023
Merged
aruniverse pushed a commit that referenced this pull request Apr 17, 2023
@mergify @swbsi
IPC blocking fix (backport #5400) [release/3.7.x] (#5404)
12a0226 
Co-authored-by: swbsi <69857376+swbsi@users.noreply.github.com>
@aruniverse aruniverse mentioned this pull request Apr 18, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmconne pmconne

@calebmshafer calebmshafer

@wgoehrig wgoehrig

@tcobbs-bentley tcobbs-bentley

@toddsouthenbentley toddsouthenbentley

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@swbsi @aruniverse @pmconne @toddsouthenbentley