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

Support SSL option for Presto connector #121

Open
toru-takahashi opened this issue Apr 20, 2019 · 0 comments
Open

Support SSL option for Presto connector #121

toru-takahashi opened this issue Apr 20, 2019 · 0 comments

Comments

@toru-takahashi
Copy link

Hello,

First of all, Thank you for supporting Presto adopter. I'm happy it because I gave up it when dmemo was released.
I have a question, so I file this issue. If you have a chance, please give me your advice.

Description

I tried our Presto environment (TreasureData), which requires SSL parameter with 443 port, to connect from dmemo.
But, I noticed that connection_config doesn't have SSL option. Then, ssl: false is always used as default in presto client library. I would like to add a new parameter to allow users to specify ssl parameter.
For example,

    def connection_config
      {
        catalog: @data_source.dbname,
        server: "#{@data_source.host}:#{@data_source.port}",
        user: @data_source.user,
        password: @data_source.password.presence,
        ssl: {verify: false},
      }.compact
    end

Question

I'm working on making a PR to support ssl option.
But, I noticed that an existing Presto user needs to migrate tables in order to add ssl option if I follow bigquery adaptor implementation. (Ex. I saw data_sources.schema and bigquery_configs.schema)

I want to minimize the code change without any table migration.

Thus, I would like to know whether the following change is acceptable for your team?

  • Host parameter allows URL like https://xxxxx/?ssl_verify=false instead of only xxxxx.
  • Then, presto_adaptor.rb parses it and build a new connection_config.

If this is acceptable, I'll try to make a PR for this.
If this is not acceptable, I'll follow a way as same as bigquery adaptor.

I'm not familiar with Rails. So, if you have any advise, that would be helpful.

Thank you,

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

1 participant