Skip to content

Conversation

ofajardo
Copy link

@ofajardo ofajardo commented Feb 12, 2019

…/schemas in the system instead of just the username.

High level description of this Pull-request

I am puzzled about the behavior of get_schema_names in dialect.py. I was expecting it to retrieve all the schemas/databases in the system but instead of that only retrieves your username.

Because I am connecting teradata to Apache superset, I needed that get_schema_names gives back the databases/schemas for SQL Lab to display them. I changed the function to look at DBC.Databases2V and it works like a charm.

I submit my changes hoping to help the package to work better.

Related Issues

#82

Reviewers

CHECKLIST:

Make sure all items are marked when you submit the pull-request.

  • Relevant documentation for functions, tests, classes, the wiki, etc. have been made

I wrote a docstring in the function, while there was none before. Not sure what else needs updating.

  • Necessary unit tests in tests/ pass with no errors

All tests fail even before my changes. I think my database is configured in a different way as the one that wrote the tests. For instance running test_dialects.py tries to write tests into a database with my username (the default database), but that's not writable in my database, I can only create Volatile tables there.
In the other hand this function is working for me when I use it in conjunction with Apache superset.
I am wondering now if the original function makes sense in the configuration of the user who wrote the tests but not for me and viceversa.

I am not sure how to proceed here. For sure start changing tests does not sound very good, maybe it's better that someone for whom the tests pass can verify that my function does not break anything. Please advise.

  • Necessary integration tests in tests/ pass with no errors

Look at the previous comment.

  • Update the CHANGELOG.md with a summary of your changes if requested

If you request so I would do.

Otto Fajardo added 2 commits February 12, 2019 15:00
@ofajardo ofajardo mentioned this pull request Feb 12, 2019
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

Successfully merging this pull request may close these issues.

1 participant