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

Problem with DB Health Provider on Oracle engine #61

Closed
jpays opened this issue Aug 25, 2021 · 2 comments
Closed

Problem with DB Health Provider on Oracle engine #61

jpays opened this issue Aug 25, 2021 · 2 comments
Assignees

Comments

@jpays
Copy link

jpays commented Aug 25, 2021

Hi,

THe DB Health provider is not working correctly with Oracle Database.

This is because the query used in the code to check if DB is up is not a valid SQL Statement for Oracle :

res = self.engine.execute(f"SELECT {expected}")

should be :

res = self.engine.execute(f"SELECT {expected} from dual")

Instead of submitting a statement which can not guaranteed to be correct for all SQLAlchemy backend, may I suggest to use the do_ping function provided in the SQL Alchemy plumbing for each dialect, which will take care of using the correct query :

Instead of :

    def get_health(self) -> DbHealthStatus:
        expected = int(time.time() * 1000)
        try:
            res = self.engine.execute(f"SELECT {expected}")
            actual = next(res)[0]
            if expected == actual:
                return DbHealthStatus(status=Status.UP, details=DbHealthDetails(self.engine.name))

            return DbHealthStatus(
                status=Status.UNKNOWN,
                details=DbHealthDetails(self.engine.name, f"Selected {expected}, got {actual}"))

        except OperationalError as e:
            return DbHealthStatus(status=Status.DOWN, details=DbHealthDetails(self.engine.name, str(e)))

do :

    def get_health(self) -> DbHealthStatus:
        try:
            conn = self.engine.raw_connection()
            if self.engine.dialect.do_ping(conn):
                return DbHealthStatus(status=Status.UP, details=DbHealthDetails(self.engine.name))

            return DbHealthStatus(
                status=Status.UNKNOWN,
                details=DbHealthDetails(self.engine.name, f"Selected {expected}, got {actual}"))

        except (OperationalError, DatabaseError) as e:
            return DbHealthStatus(status=Status.DOWN, details=DbHealthDetails(self.engine.name, str(e)))

Rgds

Jerome

@michaelyaakoby
Copy link
Member

Hi Jerome, this sounds great.
Will fix.
Thanks!

@michaelyaakoby
Copy link
Member

Hi @jpays,
Your suggestion is now merged to master and will soon be released as version 0.15.0.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants