[trino][compute] Add compute support for Trino #3837
Conversation
amitsrivastava
requested review from
athithyaaselvam,
wing2fly,
ranade1,
agl29 and
Harshg999
amitsrivastava
force-pushed
the
dev/amit/cdw-hue-trino-master
branch
from
e9e5195
to
b6432c8
Compare
desktop/core/src/desktop/management/commands/sync_warehouses.py
Outdated
Show resolved
Hide resolved
| dialect = 'hive' if t.startswith('hive') else\ | ||
| 'impala' if t.startswith('impala') else\ | ||
| 'trino' if t.startswith('trino') else None | ||
|
|
There was a problem hiding this comment.
Nit: Is formatting correct?
There was a problem hiding this comment.
pycharm likes it this way and ruff also doesn't complain so I didn't bother much. On a closer look, the L148 is actually part of the if started on L147, so an extra indent might be okay.
| @@ -1909,7 +1909,7 @@ def _get_editor(self): | |||
| 'optimizer': get_optimizer_mode(), | |||
| 'page': '/editor/?type=%(type)s' % interpreter, | |||
| 'is_sql': interpreter['is_sql'], | |||
| 'is_batchable': interpreter['dialect'] in ['hive', 'impala'] or interpreter['interface'] in ['oozie', 'sqlalchemy'], | |||
| 'is_batchable': interpreter['dialect'] in ['hive', 'impala', 'trino'] or interpreter['interface'] in ['oozie', 'sqlalchemy'], | |||
There was a problem hiding this comment.
Is this required to enable batch queries for Trino?
There was a problem hiding this comment.
If hive and impala are batch-able then why not trino? it seems to work fine on the frontend. Is there a reason not to do it?
There was a problem hiding this comment.
Even I was curious if Trino supports batch-querying in Hue OOTB. Good that the frontend is looking good.
@agl29 - Let's validate and test this out when implementing V2?
Harshg999
changed the title
amitsrivastava
force-pushed
the
dev/amit/cdw-hue-trino-master
branch
3 times, most recently
from
0182d23
to
1fdacf9
Compare
apps/beeswax/src/beeswax/conf.py
Outdated
| default_ssl_cacerts, | ||
| default_ssl_validate, | ||
| ) | ||
| from desktop.lib.conf import Config, ConfigSection, coerce_bool, coerce_csv, coerce_password_from_script | ||
|
|
||
| if sys.version_info[0] > 2: |
There was a problem hiding this comment.
Nit: Good to get rid of this py2 related conditional.
There was a problem hiding this comment.
I feel at some point in time, having an API endpoint (or few of them) in Hue for this could be beneficial.
Maybe then we don't need to depend on CDW layer running the command at a set interval and service discovery can be more dynamic and on-demand?
| @@ -1909,7 +1909,7 @@ def _get_editor(self): | |||
| 'optimizer': get_optimizer_mode(), | |||
| 'page': '/editor/?type=%(type)s' % interpreter, | |||
| 'is_sql': interpreter['is_sql'], | |||
| 'is_batchable': interpreter['dialect'] in ['hive', 'impala'] or interpreter['interface'] in ['oozie', 'sqlalchemy'], | |||
| 'is_batchable': interpreter['dialect'] in ['hive', 'impala', 'trino'] or interpreter['interface'] in ['oozie', 'sqlalchemy'], | |||
There was a problem hiding this comment.
Even I was curious if Trino supports batch-querying in Hue OOTB. Good that the frontend is looking good.
@agl29 - Let's validate and test this out when implementing V2?
With this change, trino compute can be discovered, saved to db and used similar to hive and impala.
amitsrivastava
force-pushed
the
dev/amit/cdw-hue-trino-master
branch
from
1fdacf9
to
a7e7017
Compare
With this change, trino compute can be discovered, saved to db and used similar to hive and impala.