Skip to content
This repository was archived by the owner on Feb 5, 2022. It is now read-only.

Comments

Old md-cloud fixes that should better go to master#97

Open
pereferrera wants to merge 7 commits intomasterfrom
DEV-2292
Open

Old md-cloud fixes that should better go to master#97
pereferrera wants to merge 7 commits intomasterfrom
DEV-2292

Conversation

@pereferrera
Copy link
Contributor

No description provided.

@pereferrera pereferrera requested a review from kuuurt January 20, 2022 08:35
retry_state.outcome))


def is_operational_error(e):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to fix this in order to properly catch Psql operational errors

signal.signal(signal.SIGINT, self.exit_gracefully)
signal.signal(signal.SIGTERM, self.exit_gracefully)
try:
signal.signal(signal.SIGINT, self.exit_gracefully)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These lines can only be executed if you instantiate the daemons in the main execution thread, but that is not the case in md_cloud_compute

# (see https://docs.sqlalchemy.org/en/latest/orm/session_transaction.html) # noqa: 501
t = self._get_transaction_or_raise_exception(id_)
self.session.commit()
return t
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think here the lifecycle was not handled properly so I got errors (but in sqlite it doesn't matter because there are no connections)

Copy link
Contributor

@kuuurt kuuurt left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants