[SPARK][FLINK][JAVA] http timeout fix

[pawel-big-lebowski]

Problem

Documentation says timeout should be provided in millis

But the code treats it as if it was provided in seconds

Fixes #2474

Solution

• Write failing test
• Modify the code

Note: All schema changes require discussion. Please link the issue for context.

• Your change modifies the core OpenLineage model
• Your change modifies one or more OpenLineage facets

If you're contributing a new integration, please specify the scope of the integration and how/where it has been tested (e.g., Apache Spark integration supports S3 and GCS filesystem operations, tested with AWS EMR).

One-line summary:

Checklist

• You've signed-off your work
• Your pull request title follows our guidelines
• Your changes are accompanied by tests (if relevant)
• Your change contains a small diff and is self-contained
• You've updated any relevant documentation (if relevant)
• Your comment includes a one-liner for the changelog about the specific purpose of the change (if necessary)
• You've versioned the core OpenLineage model or facets according to SchemaVer (if relevant)
• You've added a header to source files (if relevant)

---

SPDX-License-Identifier: Apache-2.0
Copyright 2018-2023 contributors to the OpenLineage project

[mobuchowski]

@pawel-big-lebowski I think we should do this other way around and make docs fit the reality rather than breaking the timeout for people using floats

[pawel-big-lebowski]

@pawel-big-lebowski I think we should do this other way around and make docs fit the reality rather than breaking the timeout for people using floats

@mobuchowski I was thinking about this as well and I think we shouldn't. Suppose there's a user who configured timeout to 10000 and lives unaware of the fact timeout settings will not work. Once we fix it, timeout will work again. If we change the docs, timeout settings for such users will be still broken.

So the tradeoff is: shall we break the contract for those who follow the docs and are unaware of the problem or shall we break the contract for those (probably a single user) who found out the cause and hacked the system?

Additionally, circuit breaker timeout also uses milliseconds.

[mobuchowski]

@pawel-big-lebowski maybe we can check if the passed value is single digit and then assume seconds? IDK, I just would not be happy if my integration stopped sending events because it started applying the 5ms timeout.

[d-m-h]

My 2 cents:

1. Add an explicit configuration (property) to specify the time unit (minutes, seconds, milliseconds)
2. Default the above time unit to whatever the behaviour is now.
3. Allow users to configure accordingly.

This way, you remove the ambiguity of "is this 5 milliseconds, or 5 seconds?"

For example:

spark.openlineage.transport.timeout=5000
spark.openlineage.transport.timeout.chrono-unit=seconds # This is the current behaviour, right?

[pawel-big-lebowski]

Nice idea from offline discussions to make as many people happy as possible

• We leave timeout as is in http config (treat it in seconds, remove it from docs), we mark config entry deprecated in the code
• We include timeoutMillis config entry and take its value for timeout if it's present

@mobuchowski would it make sense to you?

[tatiana]

Nice idea from offline discussions to make as many people happy as possible

• We leave timeout as is in http config (treat it in seconds, remove it from docs), we mark config entry deprecated in the code
• We include timeoutMillis config entry and take its value for timeout if it's present

@mobuchowski would it make sense to you?

I really like this strategy, @pawel-big-lebowski , since it is backwards compatible and also supports users specifying milliseconds. Credits to @julienledem from our chat yesterday :D