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.

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

[Oracle] Use QgsTask to retrieve available tables so we can properly cancel it #43587

Merged
merged 1 commit into from
Jul 22, 2021

Conversation

troopa81
Copy link
Contributor

@troopa81 troopa81 commented Jun 7, 2021

Fixes #40001 : Use QgsTask instead of Qthread so we can properly cancel it from QgsTaskManagerWidget and when quitting the application.

For the record, createChildren is called inside a thread which is destroyed at the end of the function, so we need to move the task to the main thread so the sent signals/events have a QEventLoop running to process them.

@troopa81 troopa81 added backport queued_ltr_backports Queued Backports Bug Either a bug report, or a bug fix. Let's hope for the latter! labels Jun 7, 2021
@github-actions github-actions bot added this to the 3.20.0 milestone Jun 7, 2021
@nyalldawson
Copy link
Collaborator

This is a GREAT move! I'd love to see the other providers doing this too at some stage.

src/providers/oracle/qgsoraclecolumntypetask.h Outdated Show resolved Hide resolved
src/providers/oracle/qgsoraclecolumntypetask.h Outdated Show resolved Hide resolved
src/providers/oracle/qgsoracledataitems.cpp Outdated Show resolved Hide resolved
src/providers/oracle/qgsoracledataitems.cpp Outdated Show resolved Hide resolved
@troopa81
Copy link
Contributor Author

@nyalldawson Everything is green, is it OK to merge?

@troopa81
Copy link
Contributor Author

@nyalldawson gentle ping, is it OK to merge?

@github-actions
Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 15, 2021
@nyalldawson nyalldawson reopened this Jul 22, 2021
@nyalldawson nyalldawson merged commit da03027 into qgis:master Jul 22, 2021
@qgis-bot
Copy link
Collaborator

The backport to queued_ltr_backports failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-queued_ltr_backports queued_ltr_backports
# Navigate to the new working tree
cd .worktrees/backport-queued_ltr_backports
# Create a new branch
git switch --create backport-43587-to-queued_ltr_backports
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 136381c8e45397d3fb118aa6491ea8f200fab12a
# Push it to GitHub
git push --set-upstream origin backport-43587-to-queued_ltr_backports
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-queued_ltr_backports

Then, create a pull request where the base branch is queued_ltr_backports and the compare/head branch is backport-43587-to-queued_ltr_backports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport queued_ltr_backports Queued Backports Bug Either a bug report, or a bug fix. Let's hope for the latter! Oracle data provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running (connection) Task makes it impossible to Quit QGIS
3 participants