You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "GumpacG (via GitHub)" <gi...@apache.org> on 2026/06/19 01:40:30 UTC

[PR] Standardized `gremlin-dotnet` connection options [tinkerpop]

GumpacG opened a new pull request, #3468:
URL: https://github.com/apache/tinkerpop/pull/3468

   # Standardize `gremlin-dotnet` connection options
   
   Implements the .NET portion of the TinkerPop 4.x GLV connection-options standardization. Renames several `ConnectionSettings` properties and the `Auth` factory methods to canonical names (old names kept as `[Obsolete]` deprecated aliases), aligns defaults, adds new options, and fixes two latent bugs. .NET driver changes only; the other GLVs follow in separate PRs.
   
   **Proposal:** https://lists.apache.org/thread/yqtr2wnb1kq2pqqq4002cz511q5o0bkg
   
   ## Renames (deprecated aliases retained)
   
   | Old                       | New              | Default |
   | ------------------------- | ---------------- | ------- |
   | `MaxConnectionsPerServer` | `MaxConnections` | 128     |
   | `IdleConnectionTimeout`   | `IdleTimeout`    | 180s    |
   | `ConnectionTimeout`       | `ConnectTimeout` | 5s      |
   | `KeepAliveInterval`       | `KeepAliveTime`  | 30s     |
   | `EnableCompression`       | `Compression`    | -       |
   | `Auth.BasicAuth`          | `Auth.Basic`     | -       |
   | `Auth.SigV4Auth`          | `Auth.Sigv4`     | -       |
   
   The old property names and auth methods are retained as `[Obsolete]` deprecated aliases (marked "As of release 4.0.0, ...") that delegate to the canonical members, so existing code keeps compiling.
   
   ## Behavior changes (breaking)
   * **`ConnectTimeout`** default lowered 15s → 5s.
   * **`KeepAliveTime`** is now wired to a real TCP keep-alive socket option (via `SocketsHttpHandler.ConnectCallback`) rather than the inert HTTP/2 ping timeout, which had no effect on HTTP/1.1.
   * **`Compression`** is now a `{None, Deflate}` enum defaulting to `Deflate` (compression on by default); the driver sends `Accept-Encoding: deflate` by default. Set `Compression.None` to disable. The deprecated `EnableCompression` `bool` is preserved only as a compatibility shim mapping `true`/`false` to `Compression.Deflate`/`Compression.None`.
   
   ## New options
   * **`Ssl`** (`SslClientAuthenticationOptions`) - adds client-certificate and custom-CA support. `SkipCertificateValidation` is applied to an internal copy rather than mutating the caller's options.
   * **`DefaultBatchSize`** (64) - connection-level default that fills the per-request `batchSize` when unset.
   * **`MaxResponseHeaderBytes`** - the maximum response header size, in bytes (converted internally to the handler's native kilobyte unit).
   * **`ReadTimeout`** - a per-read idle timeout applied to each read of the response stream (via per-read `CancelAfter`).
   * **`Proxy`** (`IWebProxy`) - routes connections through an HTTP proxy.
   * **`GremlinServer.FromUrl(string)`** / **`GremlinServer(Uri)`** - configure the endpoint from a single URL (scheme, host, port, path), deriving SSL from the scheme. The existing host/port/path constructor remains.
   
   ## Removed (breaking)
   * **`maxResponseContentLength`** - responses now stream; `ReadTimeout` is the partial mitigation.
   
   ## Bug fixes
   * Fixed deflate response decompression, which threw on the server's zlib-framed output because it used `DeflateStream` (raw DEFLATE, RFC 1951) instead of `ZLibStream` (zlib, RFC 1950). The bug was previously masked because compression was off by default; turning it on by default would have made it a default-path failure.
   * Fixed `Ssl` options cloning (used on the skip-certificate-validation path) to copy `ClientCertificateContext` and `AllowTlsResume`, which were previously dropped, breaking mTLS client certificates and silently re-enabling TLS resumption.
   
   ## Testing
   * `gremlin-dotnet` unit tests pass (695 tests), including new `ConnectionSettingsTests` and the `ReadTimeout` slow-read timeout test.
   * CHANGELOG, reference config table (`gremlin-variants.asciidoc`), and upgrade guide (`release-4.x.x.asciidoc`) updated for the .NET slice.
   
   Assisted-by: Kiro: Claude Opus 4.8
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Standardized `gremlin-dotnet` connection options [tinkerpop]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3468:
URL: https://github.com/apache/tinkerpop/pull/3468#issuecomment-4747715720

   ## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/3468?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 78.61%. Comparing base ([`a28cd1f`](https://app.codecov.io/gh/apache/tinkerpop/commit/a28cd1f2f1f10c140030a56de44c5c6c045af5f0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a18c17d`](https://app.codecov.io/gh/apache/tinkerpop/commit/a18c17d4f951b92a8072feb149aa20e873b651ae?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 120 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #3468      +/-   ##
   ============================================
   + Coverage     76.35%   78.61%   +2.26%     
   ============================================
     Files          1012       31     -981     
     Lines         60341     6202   -54139     
     Branches       7075        0    -7075     
   ============================================
   - Hits          46076     4876   -41200     
   + Misses        11548     1032   -10516     
   + Partials       2717      294    -2423     
   ```
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/tinkerpop/pull/3468?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Standardized `gremlin-dotnet` connection options [tinkerpop]

Posted by "GumpacG (via GitHub)" <gi...@apache.org>.
GumpacG commented on code in PR #3468:
URL: https://github.com/apache/tinkerpop/pull/3468#discussion_r3464559412


##########
gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionSettings.cs:
##########
@@ -85,10 +106,107 @@ public class ConnectionSettings
         /// </summary>
         public bool BulkResults { get; set; } = false;
 
+        /// <summary>
+        ///     Gets or sets the default batch size that fills the per-request batch size when it
+        ///     is not set on the request (client-side default-filling; no wire change).
+        /// </summary>
+        public int DefaultBatchSize { get; set; } = DefaultBatchSizeValue;
+
+        /// <summary>
+        ///     Gets or sets the SSL/TLS options used for HTTPS connections (client certificates,
+        ///     custom CA, protocols, etc.). When set, these options are used to configure the
+        ///     underlying handler. <see cref="SkipCertificateValidation"/> is applied to an
+        ///     internal copy of these options rather than mutating the object provided here.
+        /// </summary>
+        public SslClientAuthenticationOptions? Ssl { get; set; }
+
         /// <summary>
         ///     Gets or sets whether to skip SSL certificate validation.
-        ///     Only use for testing with self-signed certificates.
+        ///     Only use for testing with self-signed certificates. When <see cref="Ssl"/> is also
+        ///     provided, the accept-all callback is set on an internal copy of those options so the
+        ///     caller's <see cref="SslClientAuthenticationOptions"/> instance is never mutated
+        ///     (which is important when one instance is shared across multiple clients).
         /// </summary>
         public bool SkipCertificateValidation { get; set; } = false;
+
+        /// <summary>
+        ///     Gets or sets the maximum allowed size, in bytes, of the response headers.
+        ///     A value of <c>0</c> (the default) leaves the handler default unchanged. The native
+        ///     handler unit is kilobytes; the byte value provided here is converted internally.
+        /// </summary>
+        public int MaxResponseHeaderBytes { get; set; } = 0;
+
+        /// <summary>
+        ///     Gets or sets the idle-read timeout applied to each individual read of the response
+        ///     stream. It resets per chunk, so it is an idle-read timeout rather than a
+        ///     whole-request deadline. <see cref="System.Threading.Timeout.InfiniteTimeSpan"/>
+        ///     (the default) disables it.
+        /// </summary>
+        public TimeSpan ReadTimeout { get; set; } = System.Threading.Timeout.InfiniteTimeSpan;
+
+        /// <summary>
+        ///     Gets or sets the HTTP proxy used for connections. When set, it is applied to the
+        ///     underlying handler explicitly.
+        /// </summary>
+        public IWebProxy? Proxy { get; set; }

Review Comment:
   IWebProxy is the idiomatic general-purpose .NET proxy abstraction in System.Net and it's exactly what SocketsHttpHandler.Proxy consumes.
     
   Regarding other GLVs, Java is the only driver that defines a custom proxy type (ProxyOptions). The others all use their platform's native proxy representation, same as .NET does:
     - .NET: IWebProxy (BCL interface) -> handler.Proxy
     - Go: func(*http.Request) (*url.URL, error) -> http.Transport.Proxy
     - Python: proxy URL string -> aiohttp post(proxy=...)
     - JS: proxy URI string -> undici ProxyAgent
     - Java: custom ProxyOptions -> Netty HttpProxyHandler
   
   Java needed its own class because Netty is low-level (you insert an HttpProxyHandler with an address + credentials, there's no ready-made proxy object), whereas .NET/Go/Python/JS each sit on a client that already exposes a proxy type or URL. So IWebProxy keeps .NET consistent with the majority pattern (native proxy representation per language).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Standardized `gremlin-dotnet` connection options [tinkerpop]

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer merged PR #3468:
URL: https://github.com/apache/tinkerpop/pull/3468


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Standardized `gremlin-dotnet` connection options [tinkerpop]

Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on PR #3468:
URL: https://github.com/apache/tinkerpop/pull/3468#issuecomment-4802291311

   VOTE +1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Standardized `gremlin-dotnet` connection options [tinkerpop]

Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on code in PR #3468:
URL: https://github.com/apache/tinkerpop/pull/3468#discussion_r3453562000


##########
gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionSettings.cs:
##########
@@ -85,10 +106,107 @@ public class ConnectionSettings
         /// </summary>
         public bool BulkResults { get; set; } = false;
 
+        /// <summary>
+        ///     Gets or sets the default batch size that fills the per-request batch size when it
+        ///     is not set on the request (client-side default-filling; no wire change).
+        /// </summary>
+        public int DefaultBatchSize { get; set; } = DefaultBatchSizeValue;
+
+        /// <summary>
+        ///     Gets or sets the SSL/TLS options used for HTTPS connections (client certificates,
+        ///     custom CA, protocols, etc.). When set, these options are used to configure the
+        ///     underlying handler. <see cref="SkipCertificateValidation"/> is applied to an
+        ///     internal copy of these options rather than mutating the object provided here.
+        /// </summary>
+        public SslClientAuthenticationOptions? Ssl { get; set; }
+
         /// <summary>
         ///     Gets or sets whether to skip SSL certificate validation.
-        ///     Only use for testing with self-signed certificates.
+        ///     Only use for testing with self-signed certificates. When <see cref="Ssl"/> is also
+        ///     provided, the accept-all callback is set on an internal copy of those options so the
+        ///     caller's <see cref="SslClientAuthenticationOptions"/> instance is never mutated
+        ///     (which is important when one instance is shared across multiple clients).
         /// </summary>
         public bool SkipCertificateValidation { get; set; } = false;
+
+        /// <summary>
+        ///     Gets or sets the maximum allowed size, in bytes, of the response headers.
+        ///     A value of <c>0</c> (the default) leaves the handler default unchanged. The native
+        ///     handler unit is kilobytes; the byte value provided here is converted internally.
+        /// </summary>
+        public int MaxResponseHeaderBytes { get; set; } = 0;
+
+        /// <summary>
+        ///     Gets or sets the idle-read timeout applied to each individual read of the response
+        ///     stream. It resets per chunk, so it is an idle-read timeout rather than a
+        ///     whole-request deadline. <see cref="System.Threading.Timeout.InfiniteTimeSpan"/>
+        ///     (the default) disables it.
+        /// </summary>
+        public TimeSpan ReadTimeout { get; set; } = System.Threading.Timeout.InfiniteTimeSpan;
+
+        /// <summary>
+        ///     Gets or sets the HTTP proxy used for connections. When set, it is applied to the
+        ///     underlying handler explicitly.
+        /// </summary>
+        public IWebProxy? Proxy { get; set; }

Review Comment:
   Maybe too specific to the HttpClient? It seems like some other GLVs create specific options.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Standardized `gremlin-dotnet` connection options [tinkerpop]

Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on PR #3468:
URL: https://github.com/apache/tinkerpop/pull/3468#issuecomment-4813940967

   VOTE +1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org