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

2024-10-11-fix-rollback_session #167

Merged
merged 11 commits into from
Oct 14, 2024
Merged

Conversation

sjib
Copy link
Contributor

@sjib sjib commented Oct 11, 2024

add missing function rollback_session

Should fix QGEP/QGEP#873

@sjib sjib added the fix label Oct 11, 2024
@sjib sjib self-assigned this Oct 11, 2024
@sjib
Copy link
Contributor Author

sjib commented Oct 11, 2024

@ponceta Does this look like fixing the problem?

Could we eliminate the error messages in this block of commit_session?


        try:
            self.session.commit()
        except Exception as e:
            self.session.rollback_session()
            iface.messageBar().pushMessage("Error", f"An error occurred: {e}", level=Qgis.Warning)
            iface.messageBar().pushMessage("Error", "Import was canceled", level=Qgis.Warning)
        finally:
            self.session.close()

Or better keep them there and only do

    def rollback_session(self):
        self.session.rollback()

and not messages, as they are a kind of duplicates?

Thanks for reviewing

@sjib sjib requested review from ponceta and domi4484 October 11, 2024 12:58
@ponceta
Copy link
Member

ponceta commented Oct 11, 2024

This would be better reviewed by @domi4484 but IMHO it should be best to add rollback messages directly in the rollback function.

@ponceta
Copy link
Member

ponceta commented Oct 11, 2024

I think you have to use the function you defined and not self.session.rollback_session()

    except Exception as e:
        self.rollback_session()
        iface.messageBar().pushMessage("Error", f"An error occurred: {e}", level=Qgis.Warning)
        iface.messageBar().pushMessage("Error", "Import was canceled", level=Qgis.Warning)

@sjib
Copy link
Contributor Author

sjib commented Oct 12, 2024

@ponceta better now?

@ponceta ponceta merged commit 91e152b into master Oct 14, 2024
5 checks passed
@ponceta ponceta deleted the 2024-10-11-fix-rollback_session branch October 14, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

name 'rollback_session' is not defined
2 participants