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

BUG: fix datamgr.py fail if IBIS_TEST_OMNISCIDB_DATABASE=omnisci #2057

Merged
merged 3 commits into from
Feb 2, 2020

Conversation

anmyachev
Copy link
Contributor

closes #2049

Copy link
Contributor

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

@anmyachev good catch! and thanks for work on that.

maybe you can refactor that a little bit:

  • you can remove the else statement you added and
  • add a similar code before if (between line 348 and 349)
  • and remove database = params["database"] from line 358

eg:

    default_db = 'omnisci'
    database = params["database"]
    
    if params['database'] != default_db:
        conn = pymapd.connect(
            host=params['host'],
            user=params['user'],
            password=params['password'],
            port=params['port'],
            dbname=default_db,
            protocol=params['protocol'],
        )
        stmt = "DROP DATABASE IF EXISTS {}".format(database)
        try:
            conn.execute(stmt)
        except Exception:
            logger.warning('OmniSci DDL statement %r failed', stmt)

        stmt = 'CREATE DATABASE {}'.format(database)
        try:
            conn.execute(stmt)
        except Exception:
            logger.exception('OmniSci DDL statement %r failed', stmt)
        conn.close()

    conn = pymapd.connect(
        host=params['host'],
        user=params['user'],
        password=params['password'],
        port=params['port'],
        dbname=database,
        protocol=params['protocol'],
    )

@anmyachev
Copy link
Contributor Author

@xmnlab the PR ready for review

Copy link
Contributor

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

LGTM! thanks for working on that @anmyachev

@xmnlab xmnlab changed the title [BUG] fix datamgr.py fail if IBIS_TEST_OMNISCIDB_DATABASE=omnisci BUG: fix datamgr.py fail if IBIS_TEST_OMNISCIDB_DATABASE=omnisci Feb 1, 2020
@xmnlab
Copy link
Contributor

xmnlab commented Feb 1, 2020

@anmyachev could you add a release note for that pls? thanks!

@xmnlab xmnlab self-assigned this Feb 1, 2020
@xmnlab xmnlab added bug Incorrect behavior inside of ibis omnisci labels Feb 1, 2020
@anmyachev
Copy link
Contributor Author

@xmnlab release note was be added. Thanks for review!

@xmnlab xmnlab merged commit 2f24d3b into ibis-project:master Feb 2, 2020
@xmnlab
Copy link
Contributor

xmnlab commented Feb 2, 2020

Thanks @anmyachev !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior inside of ibis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

datamgr.py fails if IBIS_TEST_OMNISCIDB_DATABASE=omnisci
2 participants