Enable Impala impersonation by passing cursor configuration.#4699
Enable Impala impersonation by passing cursor configuration.#4699tothandor wants to merge 1 commit into
Conversation
This commit will only work if pull request apache#298 at cloudera/impyla is accepted, but it may not do no harm if not. It has also touched Hive impersonation, though it should not affect it.
mistercrunch
left a comment
mistercrunch
left a comment
There was a problem hiding this comment.
Overall looks sound except for a minor comment. Please add a unit test as this part of the codebase is pretty scattered and engine-specific. I could see how adding impersonation for other engines could create regressions if this is not protected by tests.
Also the build is failing because of minor linting issues, check out the travis logs for details.
| logging.info('Database.get_sqla_engine(). Masked URL: {0}'.format(masked_url)) | ||
|
|
||
| params = extra.get('engine_params', {}) | ||
| self.cursor_kwargs = self.db_engine_spec.get_cursor_configuration_for_impersonation(str(url), self.impersonate_user, effective_username) |
There was a problem hiding this comment.
Stamping into self.cursor_kwargs here is brittle as it assumes method call ordering which isn't an explicit assumption. Either make this happen on the __init__ method if the context is available at that point, or use a @property.
|
Hi, Did you have this issue as well @tothandor ? |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
4 participants
This commit will only work if pull request #298 at cloudera/impyla is accepted,
but it may not do no harm if not. ImpalaEngineSpec.get_schema_names() had to be moved to cloudera/impyla-s SQLAlchemy API, in order to be able to get an impersonated cursor for the inspector.
It has also touched Hive impersonation, though it should not affect it.
It works for me, but feel free to completly rewrite it, to make it fit better in the overall picture.