Add global proxy setting #51749

arthurpassos · 2023-07-03T19:50:58Z

Changelog category (leave one):

Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Attempt to create a generic proxy resolver for CH while keeping backwards compatibility with existing S3 storage conf proxy resolver.

Adds a fallback proxy resolver which is based on the http_proxy and https_proxy environment variables.

Make S3 table functions respect it. URL functions as well.

Closes #14097

Documentation entry for user-facing changes

Adds new global proxy setting with new syntax. Adds fallback proxy resolver based on http_proxy and https_proxy environment variables.

Documentation is written (mandatory for new features)

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

…olverProvider

tests/integration/test_s3_table_function_with_proxy/test.py

robot-ch-test-poll · 2023-07-05T22:12:29Z

This is an automated comment for commit 2367434 with description of existing statuses. It's updated for the latest CI running
The full report is available here
The overall status of the commit is 🔴 failure

Check name	Description	Status
AST fuzzer	Runs randomly generated queries to catch program errors. The build type is optionally given in parenthesis. If it fails, ask a maintainer for help	🟢 success
CI running	A meta-check that indicates the running CI. Normally, it's in success or pending state. The failed status indicates some problems with the PR	🟢 success
ClickHouse build check	Builds ClickHouse in various configurations for use in further steps. You have to fix the builds that fail. Build logs often has enough information to fix the error, but you might have to reproduce the failure locally. The cmake options can be found in the build log, grepping for cmake. Use these options and follow the general build process	🟢 success
Compatibility check	Checks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help	🟢 success
Docker image for servers	The check to build and optionally push the mentioned image to docker hub	🟢 success
Fast test	Normally this is the first check that is ran for a PR. It builds ClickHouse and runs most of stateless functional tests, omitting some. If it fails, further checks are not started until it is fixed. Look at the report to see which tests fail, then reproduce the failure locally as described here	🟢 success
Flaky tests	Checks if new added or modified tests are flaky by running them repeatedly, in parallel, with more randomization. Functional tests are run 100 times with address sanitizer, and additional randomization of thread scheduling. Integrational tests are run up to 10 times. If at least once a new test has failed, or was too long, this check will be red. We don't allow flaky tests, read the doc	🟢 success
Install packages	Checks that the built packages are installable in a clear environment	🟢 success
Integration tests	The integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests	🔴 failure
Mergeable Check	Checks if all other necessary checks are successful	🟢 success
Performance Comparison	Measure changes in query performance. The performance test report is described in detail here. In square brackets are the optional part/total tests	🟢 success
Push to Dockerhub	The check for building and pushing the CI related docker images to docker hub	🟢 success
SQLTest	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	🟢 success
SQLancer	Fuzzing tests that detect logical bugs with SQLancer tool	🟢 success
Sqllogic	Run clickhouse on the sqllogic test set against sqlite and checks that all statements are passed	🟢 success
Stateful tests	Runs stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	🟢 success
Stateless tests	Runs stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	🟢 success
Stress test	Runs stateless functional tests concurrently from several clients to detect concurrency-related errors	🟢 success
Style Check	Runs a set of checks to keep the code style clean. If some of tests failed, see the related log from the report	🟢 success
Unit tests	Runs the unit tests for different release types	🟢 success
Upgrade check	Runs stress tests on server version from last release and then tries to upgrade it to the version from the PR. It checks if the new server can successfully startup without any errors, crashes or sanitizer asserts	🟢 success

arthurpassos · 2023-07-06T12:59:41Z

The downside of this approach is that everytime a HTTP request is created, one has to remember to grab & pass proxy config. I feel like this should be hardcoded within the HTTP request "creator". Maybe it can be done for the generic case (i.e the non-S3 specific configuration we are adding with this PR), but it will not work for the existing storage conf one.

One idea that could work is to indeed hardcode the proxy logic within the HTTP request creator which is applied if the proxy config argument hasn't been specified.

Not sure if these are feasible, will read the code and come back.

arthurpassos · 2023-08-21T16:36:31Z

@tavplubix kind ping

tavplubix · 2023-08-21T16:44:04Z

@arthurpassos, I'm reviewing it right now.

tests/integration/test_s3_storage_conf_proxy/test.py

src/Common/EnvironmentProxyConfigurationResolver.cpp

tavplubix · 2023-08-21T16:11:42Z

src/Common/ProxyConfiguration.h

+struct ProxyConfiguration
+{
+    enum class Protocol
+    {
+        HTTP,
+        HTTPS,
+        ANY


So the only difference from ClientConfigurationPerRequest is support for ANY, right? But does it really make sense if we handle ANY as HTTP?

That's not the only difference. The main and most important difference is that ClientConfigurationPerRequest is a S3 specific structure and relies on Aws::Http::Scheme (a third party module enum).

ProxyConfiguration is a ClickHouse domain enum, meant to be used across ClickHouse source code. It removes this extra dependency and tight coupling.

For instance, URL functions also need proxy, but should not have a dependency on AWS stuff. There might be other parts of CH that depend on proxy stuff and will adhere to this implementation later, no dependency on AWS stuff.

Not only that, but ProxyConfiguration::Protocol::ANY is not handled as HTTP. Both RemoteResolver and ListResolver will literally use ANY (HTTP or HTTPs) if ANY is specified. You can check that in ProxyConfigurationResolverProvider.cpp.

You might have been confused by the EnvironmentResolver, which is not currently handling ANY as ANY. I'll address that.

ClientConfigurationPerRequest is a S3 specific structure and relies on Aws::Http::Scheme (a third party module enum)

I don't mind using Aws::Http::Scheme in ClickHouse code (since we already do), but okay

You might have been confused by the EnvironmentResolver, which is not currently handling ANY as ANY

Yes, and also by

ClickHouse/src/IO/S3/ProxyConfigurationResolverAdapter.cpp

Lines 18 to 20 in 63c7ba5

case DB::ProxyConfiguration::Protocol::ANY:

// default to HTTP since there is no ANY in AWS::Scheme and we don't want an exception

return Aws::Http::Scheme::HTTP;

However, if I'm not mistaken, ANY is used only as an argument of a factory (to get any proxy) and is never used as a part of an actual proxy configuration (each proxy is either HTTP or HTTPS). In other words, can ProxyConfiguration::protocol be ANY?

Yes, ANY is only used to get any resolver, but ProxyConfiguration::protocol will never be ANY.

The only reason ANY exists is to keep backwards compatibility with the "old" behavior, where the user did not have the option to specify if the proxy was meant for HTTP or for HTTPs requests. ClickHouse would pick any.

So ANY is only used in the context of the S3 storage conf proxy.

Yes, ANY is only used to get any resolver, but ProxyConfiguration::protocol will never be ANY.

So there's no real difference between ClientConfigurationPerRequest and ProxyConfiguration (except for using a third party module enum)

Yes, you are correct

src/IO/S3/ProxyConfigurationProvider.h

tavplubix · 2023-08-21T17:00:23Z

src/IO/S3/ProxyConfigurationResolverAdapter.h

+class ProxyConfigurationResolverAdapter : public ProxyConfiguration
+{
+public:
+    explicit ProxyConfigurationResolverAdapter(std::shared_ptr<ProxyConfigurationResolver> resolver_)
+        : resolver(resolver_) {}
+    /// Returns proxy configuration on each HTTP request.
+    ClientConfigurationPerRequest getConfiguration(const Aws::Http::HttpRequest & request) override;
+    void errorReport(const ClientConfigurationPerRequest & config) override;


Either I'm missing something or it should have failed to compile. The base class, ProxyConfiguration is a trivial struct with no virtual methods, and there are no other base classes. So there's nothing to override, these methods are not virtual

Also, can we get rid of ProxyConfigurationResolverAdapter at all? As far as I understood, ClientConfigurationPerRequest to ProxyConfiguration and vice versa (and the only difference between them is ANY, but there's another comment about this).

There are only two usages of ProxyConfigurationProvider with ProxyConfigurationResolverAdapter that look like

auto proxy_configuration_resolver = S3::ProxyConfigurationProvider::get(protocol); auto per_request_configuration = [=] (const Aws::Http::HttpRequest & req) { return proxy_configuration_resolver->getConfiguration(req); }; auto error_report = [=] (const ClientConfigurationPerRequest & req) { proxy_configuration_resolver->errorReport(req); };

Let's just add two simple methods like ProxyConfiguration::toClientConfiguration() and ProxyConfiguration::fromClientConfiguration(...). AFAIU, it will allow to replace the adapter with simple code like:

auto proxy_configuration_resolver = S3::ProxyConfigurationResolverProvider::get(protocol); auto per_request_configuration = [=] (const Aws::Http::HttpRequest & req) { return proxy_configuration_resolver->resolve().toClientConfiguration(); }; auto error_report = [=] (const ClientConfigurationPerRequest & req) { proxy_configuration_resolver->errorReport(ProxyConfiguration::fromClientConfiguration(req)); };

Or we can even override the implicit conversion operator...

Either I'm missing something or it should have failed to compile. The base class, ProxyConfiguration is a trivial struct with no virtual methods, and there are no other base classes. So there's nothing to override, these methods are not virtual

This is confusing because of backwards compatibility. Previously, proxy support was added only to S3 storage. At that moment, the DB::S3::ProxyConfiguration class was introduced. See https://github.com/ClickHouse/ClickHouse/blob/master/src/Disks/ObjectStorages/S3/ProxyConfiguration.h. This is the base class that contains those methods.

For me, it's poorly named. It's named ProxyConfiguration, but it's not a configuration, it's a class that will provide / resolve / generate the configuration.

On the other hand, the DB::ProxyConfiguration, struct you are seeing on this PR, is an actual configuration class.

So, to summarize, DB::S3::ProxyConfiguration is a poorly named class and serves as a resolver for S3 proxy configuration. DB::ProxyConfiguration is an actual configuration class mean to be used across ClickHouse without any dependency (i.e, can be used for URL, S3, and whatever else).

Also, can we get rid of ProxyConfigurationResolverAdapter at all? As far as I understood, ClientConfigurationPerRequest to ProxyConfiguration and vice versa (and the only difference between them is ANY, but there's another comment about this).

There are only two usages of ProxyConfigurationProvider with ProxyConfigurationResolverAdapter that look like

auto proxy_configuration_resolver = S3::ProxyConfigurationProvider::get(protocol); auto per_request_configuration = [=] (const Aws::Http::HttpRequest & req) { return proxy_configuration_resolver->getConfiguration(req); }; auto error_report = [=] (const ClientConfigurationPerRequest & req) { proxy_configuration_resolver->errorReport(req); };

Let's just add two simple methods like ProxyConfiguration::toClientConfiguration() and ProxyConfiguration::fromClientConfiguration(...). AFAIU, it will allow to replace the adapter with simple code like:

auto proxy_configuration_resolver = S3::ProxyConfigurationResolverProvider::get(protocol); auto per_request_configuration = [=] (const Aws::Http::HttpRequest & req) { return proxy_configuration_resolver->resolve().toClientConfiguration(); }; auto error_report = [=] (const ClientConfigurationPerRequest & req) { proxy_configuration_resolver->errorReport(ProxyConfiguration::fromClientConfiguration(req)); };

Or we can even override the implicit conversion operator...

I can do something along these lines for the sake of the PR, but I honestly prefer to avoid throwing the "burden" of converting stuff and "knowing implementation details" to the client. As of now, for the client, it's seamless. It does not know there are indirection layers, conversions and where the proxy configuration is coming from. If that changes, client code will not have to change.

I also don't like the idea of having a DB::ProxyConfiguration constructor that takes ClientConfiguration or the other way around. This adds extra dependencies.

I usually code with the notion of "the less a class knows, the better". If URL functions need to have access to DB::ProxyConfiguration, it should only see that. With those auxiliary methods or that constructor, it'll "see" DB::S3::ClientConfiguration, a useless dependency for URL functions (or anything outside the S3 scope).

I do understand though it's hard to keep track of a big number of classes and it might be confusing because of the similar names. Sometimes namespaces are not enough for us to keep track of that.

Let me know what you think.

"knowing implementation details" to the client. As of now, for the client, it's seamless

The client is basically 6 lines of code in 2 places (3 lines in each). Updating these 6 lines is much simpler than introducing some abstract adapters (about 100 lines of code).

If that changes, client code will not have to change.

If that changes, we can handle changing 6 lines of code.

This adds extra dependencies.

It's not a big problem (we have a monorepo and a static binary). Also, it's quite natural to have an easy way to convert "new" ProxyConfiguration to "old" ClientConfiguration and vice versa

I usually code with the notion of "the less a class knows, the better"

I understand this, and it's a really good principle sometimes. But I also like "less code is better" and "simpler is better". And in this particular case, all these abstractions look to me a bit overengineered (or maybe like "premature optimization", but in terms of abstractions)

With those auxiliary methods or that constructor, it'll "see" DB::S3::ClientConfiguration, a useless dependency for URL functions (or anything outside the S3 scope).

I don't think it's a big problem, especially if URL function will not explicitly use this stuff and will only include some headers containing it. But if you want to do it the right way, then the right way is probably different.

S3 is basically the same as URL with something extra on top of basic HTTP(S). S3 and URL should depend on some common http stuff. And proxies must be a part of that common http stuff, it's not something S3-specific. So the right way is to refactor the code in the S3 scope, and replace DB::S3::ClientConfiguration with something that can be used in both URL and S3

ok, fair enough. I'll see what I can do about this refactor, worst case scenario I'll add the auxiliary functions

Actually, it's not that simple. There is extra logic implemented in DB::S3::ProxyConfigurationProvider, this extra logic is required to keep backwards compatibility.

To integrate the old behavior / settings with the new behavior / settings, here's what's implemented:

Try to fetch from scoped settings (storage conf proxy specific).

If it can't find storage conf specific proxy settings, try to fetch from general settings.

If it can't find there, grab from env.

This is the job of DB::S3::ProxyConfigurationProvider.

If I manage to remove ClientConfigurationPerRequest and use DB::ProxyConfiguration only, then maybe it's ok to eliminate DB::S3::ProxyConfigurationResolverAdapter and DB::ProxyConfigurationProvider by adding a method a backwards compatible method in DB::ProxyConfigurationResolverProvider. Will look into it

Removed ClientConfigurationPerRequest, DB::S3::ProxyConfigurationProvider, DB::S3::ProxyConfiguration and DB::S3::ProxyConfigurationResolverAdapter.

Can you double check it's ok now?

Thanks, will review it tomorrow

src/IO/S3/ProxyConfigurationResolverAdapter.cpp

src/Common/ProxyConfigurationResolverProvider.cpp

src/Common/EnvironmentProxyConfigurationResolver.cpp

tavplubix · 2023-08-21T22:39:18Z

src/Common/ProxyConfiguration.h

+struct ProxyConfiguration
+{
+    enum class Protocol
+    {
+        HTTP,
+        HTTPS,
+        ANY


ClientConfigurationPerRequest is a S3 specific structure and relies on Aws::Http::Scheme (a third party module enum)

I don't mind using Aws::Http::Scheme in ClickHouse code (since we already do), but okay

You might have been confused by the EnvironmentResolver, which is not currently handling ANY as ANY

Yes, and also by

ClickHouse/src/IO/S3/ProxyConfigurationResolverAdapter.cpp

Lines 18 to 20 in 63c7ba5

case DB::ProxyConfiguration::Protocol::ANY:

// default to HTTP since there is no ANY in AWS::Scheme and we don't want an exception

return Aws::Http::Scheme::HTTP;

tavplubix · 2023-08-21T22:46:08Z

src/Common/ProxyConfigurationResolverProvider.cpp

+            if (resolver_configs > 2)
+            {
+                throw Exception(ErrorCodes::BAD_ARGUMENTS, "Only two remote proxy resolvers are allowed, one for HTTP and one for HTTPs");


Looks like it allows having two http (or two https) proxies configured, but doesn't allow, for example, 2 http + 1 https

Hm.. indeed. Correct behavior should be: 1 HTTP and 1 HTTPs. Will fix that tomorrow

@tavplubix what do you think about allowing the user to specify N HTTP remote resolvers and N HTTPs remote resolvers, but we only pick the first one of each and simply ignore the rest. It'll make code a lot simpler.

Another thing that might be worth discussing is that these exceptions are not thrown at startup time, rather upon request.

In this case, the request will be aborted. Should we try the other proxy methods first (list and environment)?

allowing the user to specify N HTTP remote resolvers and N HTTPs remote resolvers, but we only pick the first one of each and simply ignore the rest.

It's okay

Another thing that might be worth discussing is that these exceptions are not thrown at startup time, rather upon request.

It's okay that requests will fail if the server is misconfigured (it would be better to check on startup, but it's not necessary)

Should we try the other proxy methods first (list and environment)?

No, we should not try to continue working if the server is misconfigured

Allowing the user to specify N HTTP remote resolvers and N HTTPs remote resolvers, but we only pick the first one of each and simply ignore the rest.

It's okay

Ok, removed the hard check for only two resolvers. It now simply picks the first configuration that matches the protocol.

Should we try the other proxy methods first (list and environment)?

No, we should not try to continue working if the server is misconfigured

If it is misconfigured, it'll throw an exception. No fallthrough. The only caveat is "missing protocol". For instance, if the user configured a remote resolver (has precedence over other methods) for HTTP, but not for HTTPs. And there is a list resolver configured for HTTPs, it'll grab the list resolver. I believe that's acceptable, right? It's more permissive and less checks are present in the code

arthurpassos · 2023-08-23T16:23:22Z

@tavplubix I believe all comments have been addressed. I am considering to add support for no_proxy, but will most likely implement it in a different PR

arthurpassos · 2023-08-24T12:03:11Z

@tavplubix CI is green and you have approved it, can we merge it?

tavplubix · 2023-08-24T13:06:27Z

Integration tests (asan) [1/6] - #51464
Integration tests (asan, analyzer) [5/6] - #53323

arthurpassos · 2023-08-24T13:14:09Z

@tavplubix thanks for looking into this, really appreciate it

alexey-milovidov · 2023-09-01T14:35:59Z

Test has failed: https://s3.amazonaws.com/clickhouse-test-reports/54045/c25dd560c9a06312b8267906e4847560a8d29fd7/integration_tests__release__[2_4].html

This reverts commit 2bade7d.

arthurpassos added 6 commits July 3, 2023 16:36

initial impl

3c0e962

fix env ut

187399c

move ut directory

d5a6728

make sure no null proxy resolver is returned by ProxyConfigurationRes…

4162b34

…olverProvider

minor adjustment

615439d

add a few tests, still incomplete

bce8650

arthurpassos commented Jul 5, 2023

View reviewed changes

tests/integration/test_s3_table_function_with_proxy/test.py Outdated Show resolved Hide resolved

arthurpassos added 2 commits July 5, 2023 13:11

add proxy support for url table function

c3ba44a

use proxy for select from url as well

3998a37

den-crane added the can be tested Allows running workflows for external contributors label Jul 5, 2023

robot-ch-test-poll added the pr-improvement Pull request with some product improvements label Jul 5, 2023

remove optional from return type, just returns empty config

42f380a

arthurpassos added 16 commits July 6, 2023 16:41

fix style

cc4807a

style

5dc88aa

black

47ed873

ohg boy

1bfd379

rm in progress file

3da40ef

god pls don't let me kill anyone

cbfa2a0

...

087e00e

add use_aws guards

3f5aceb

Merge branch 'master' into add_global_proxy_setting

fd132b4

remove hard coded s3 proxy resolver

9a2c576

add concurrency-mt-unsafe

5508575

aa

0f94457

black

fb39e3e

add logging back

39de342

revert change

607f58b

imrpove code a bit

b011172

tavplubix reviewed Aug 21, 2023

View reviewed changes

fix env proxy not respecting any

63c7ba5

tavplubix reviewed Aug 21, 2023

View reviewed changes

arthurpassos added 13 commits August 22, 2023 10:19

use log trace

1709d85

fix wrong logic in getRemoteREsolver

0dba0b2

fix wrong logic in getRemoteREsolver

8bd1a8b

fix test

71dcf8e

remove unwanted code

f487f87

remove ClientConfigurationperRequest and auxilary classes

17e1e4b

remove unwanted code

4588bc1

remove adapter test

3c6bbd3

few adjustments and add test for s3 storage conf with new proxy settings

cd089ed

Merge branch 'master' into add_global_proxy_setting

447a671

black

4e67c8c

use chassert for context

850acc6

Add getenv comment

2367434

tavplubix approved these changes Aug 23, 2023

View reviewed changes

tavplubix merged commit 2bade7d into ClickHouse:master Aug 24, 2023
273 of 275 checks passed

alexey-milovidov added a commit that referenced this pull request Sep 1, 2023

Revert "Add global proxy setting (#51749)"

6b8bba1

This reverts commit 2bade7d.

alexey-milovidov mentioned this pull request Sep 1, 2023

Revert "Add global proxy setting (#51749)" #54182

Closed

arthurpassos mentioned this pull request Oct 3, 2023

Add option to set env variables for a single node in integration tests #55208

Merged

1 task

lesandie mentioned this pull request Feb 15, 2024

New URL scheme for S3 VPCE endpoints #60021

Closed

dubinny-anton mentioned this pull request Jun 5, 2024

Support usage of global proxy configuration for disk storage of 'web' type #64859

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add global proxy setting #51749

Add global proxy setting #51749

arthurpassos commented Jul 3, 2023 •

edited

robot-ch-test-poll commented Jul 5, 2023 •

edited by robot-clickhouse

arthurpassos commented Jul 6, 2023

arthurpassos commented Aug 21, 2023

tavplubix commented Aug 21, 2023

tavplubix Aug 21, 2023

arthurpassos Aug 21, 2023

arthurpassos Aug 21, 2023

tavplubix Aug 21, 2023

tavplubix Aug 21, 2023

arthurpassos Aug 21, 2023

tavplubix Aug 22, 2023

arthurpassos Aug 22, 2023

tavplubix Aug 21, 2023

tavplubix Aug 21, 2023

arthurpassos Aug 21, 2023

arthurpassos Aug 21, 2023

tavplubix Aug 21, 2023

arthurpassos Aug 21, 2023

arthurpassos Aug 22, 2023

arthurpassos Aug 22, 2023

tavplubix Aug 22, 2023

tavplubix Aug 21, 2023

tavplubix Aug 21, 2023

arthurpassos Aug 22, 2023

arthurpassos Aug 22, 2023

arthurpassos Aug 22, 2023

tavplubix Aug 22, 2023

arthurpassos Aug 23, 2023 •

edited

arthurpassos commented Aug 23, 2023

arthurpassos commented Aug 24, 2023

tavplubix commented Aug 24, 2023

arthurpassos commented Aug 24, 2023

alexey-milovidov commented Sep 1, 2023

	case DB::ProxyConfiguration::Protocol::ANY:
	// default to HTTP since there is no ANY in AWS::Scheme and we don't want an exception
	return Aws::Http::Scheme::HTTP;

Add global proxy setting #51749

Add global proxy setting #51749

Conversation

arthurpassos commented Jul 3, 2023 • edited

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Documentation entry for user-facing changes

robot-ch-test-poll commented Jul 5, 2023 • edited by robot-clickhouse

arthurpassos commented Jul 6, 2023

arthurpassos commented Aug 21, 2023

tavplubix commented Aug 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arthurpassos Aug 23, 2023 • edited

Choose a reason for hiding this comment

arthurpassos commented Aug 23, 2023

arthurpassos commented Aug 24, 2023

tavplubix commented Aug 24, 2023

arthurpassos commented Aug 24, 2023

alexey-milovidov commented Sep 1, 2023

arthurpassos commented Jul 3, 2023 •

edited

robot-ch-test-poll commented Jul 5, 2023 •

edited by robot-clickhouse

arthurpassos Aug 23, 2023 •

edited