fix(docs): respect no_proxy environment variable

[karsten-wagner]

SUMMARY

The clickhouse-connect library 0.4.1 did not handle a no_proxy environment variable. This is eventually problematic when using the Docker Compose Installation Guide in an environment that requires a forward proxy (e.g., in an enterprise environment) and where Clickhouse should be used as a database. In such case the call to the database should remain inside the docker network to allow addressing the database via its container name. Calls to external data sources should be rerouted via the forward proxy. Version 0.4.1 handles all connections via the proxy and thus misses the local network connection inside docker network and thus eventually fails host name resolution, given the forward proxy sits outside the docker network and is not aware of the inside docker network container names.

The issue has been fixed in the clickhouse-connect library in version 0.5.19 via #PR163, hence bumping version here in the superset docs accordingly.

TESTING INSTRUCTIONS

• Follow the Docker Compose Installation Guide
• Follow the existing Clickhouse Installation Guide referencing version 0.4.1
• Add a clickhouse container named clickhouse to docker-compose-non-dev.yml, e.g.:

  x-clickhouse-volumes:
    &clickhouse-volumes
    - clickhouse_home:/var/lib/clickhouse
  
  clickhouse:
    image: clickhouse/clickhouse-server:23
    container_name: clickhouse
    env_file: .env
    user: "root"
    restart: unless-stopped
    ports:
      - "8123:8123"
      - "9003:9000"
    volumes: *clickhouse-volumes
  
  volumes:
    clickhouse_home:
      external: false

• Update docker/.env to contain http_proxy, https_proxy and no_proxy environment variables, e.g.:

  # Local Proxy Settings
  http_proxy="http://host.docker.internal:8888"
  https_proxy="http://host.docker.internal:8888"
  no_proxy="localhost,127.0.0.1,db,redis,superset,superset-init,superset-worker,superset-beat,clickhouse"

• Provide the forward proxy on the host OS (e.g., Fiddler - in above example on port 8888)
• Start all containers and log in to superset
• Try to add Clickhouse database via connection string clickhousedb://clickhouse/default

The connection will fail. See eventually logs in the ´superset_app` container. Superset UI shows an error message on the dialog with the connection string, but not on the one where you provide host, port etc. in separate fields.

• Change docker/requirements-local.txt to contain new version clickhouse-connect>=0.5.19
• Repeat the test to add the database and all works

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[karsten-wagner]

Big thanks to @genzgd for all the hard work to respect the no_proxy variable in the clickhouse-connect library. I am just connecting the dots here...

[genzgd]

I'd recommend using 0.5.20 as the minimum, since it has a somewhat important bug fix concerning HTTP headers.

[genzgd]

Also big thanks for adding the this PR!

[github-actions]

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

[karsten-wagner]

@genzgd, updated to ´0.5.20´ as suggested.

[genzgd]

Just a heads up, at this point the current version is 0.6.8 and 0.6.x is required for Superset 2.1.+

[karsten-wagner]

@michael-s-molina, anything missing to merge this to master?

[michael-s-molina]

LGTM

[michael-s-molina]

@michael-s-molina, anything missing to merge this to master?

I was just waiting for CI to complete 😄

Thank you for the fix @karsten-wagner and for the awesome PR description!

[karsten-wagner]

Thanks and welcome! Superset is a great tool!