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

#52: Added CLI options for specifying the timeout for the ExtractValidator #57

Merged
merged 14 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/changes/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ This enables faster and more robust tests for the pure CLI-related features, fas

* #50: Created new implementation `ExtractValidator` for validating extraction of
* #49: Integrated new `ExtractValidator` into `LanguageContainerDeployer`
* #52: Added timeout options for SLC deployer to CLI

# Refactoring

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ def get_udf_path(bucket_base_path: bfs.path.PathLike, bucket_file: str) -> PureP
return PurePosixPath(file_path.as_udf_path())


def display_extract_progress(n: int, pending: List[int]):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How often would this be called?

Copy link
Contributor Author

@ckunki ckunki Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends on the settings.

  • timeout by default is now updated to 10 minutes, can be changed via CLI
  • interval currently, is always set to 10 seconds, not configurable via CLI

So with these setting, by default the callback will be called 10*60/10 = 60 times at max.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably, would increase the interval to 30 seconds

Copy link
Contributor Author

@ckunki ckunki Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - changed in latest push.

logger.info(f"Verify extraction: {len(pending)} of {n} nodes pending, IDs: {pending}")


class LanguageContainerDeployer:

def __init__(self,
Expand Down Expand Up @@ -306,7 +310,10 @@ def create(cls,
path_in_bucket: str = '',
use_ssl_cert_validation: bool = True, ssl_trusted_ca: Optional[str] = None,
ssl_client_certificate: Optional[str] = None,
ssl_private_key: Optional[str] = None) -> "LanguageContainerDeployer":
ssl_private_key: Optional[str] = None,
extract_timeout: timedelta = timedelta(seconds=10),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A proper value for a timeout is beyond 10 minutes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will set it to 10 minutes, see next push.

display_progress: bool = False,
) -> "LanguageContainerDeployer":

# Infer where the database is - on-prem or SaaS.
if all((dsn, db_user, db_password, bucketfs_host, bucketfs_port,
Expand Down Expand Up @@ -359,4 +366,6 @@ def create(cls,
encryption=True,
websocket_sslopt=websocket_sslopt)

return cls(pyexasol_conn, language_alias, bucketfs_path)
callback = display_extract_progress if display_progress else None
extract_validator = ExtractValidator(pyexasol_conn, extract_timeout, callback=callback)
return cls(pyexasol_conn, language_alias, bucketfs_path, extract_validator)
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from pathlib import Path
import click
from exasol.python_extension_common.deployment.language_container_deployer import LanguageContainerDeployer
from datetime import timedelta


class CustomizableParameters(Enum):
Expand Down Expand Up @@ -152,6 +153,9 @@ def secret_callback(ctx: click.Context, param: click.Option, value: Any):
@click.option('--alter-system/--no-alter-system', type=bool, default=True)
@click.option('--allow-override/--disallow-override', type=bool, default=False)
@click.option('--wait_for_completion/--no-wait_for_completion', type=bool, default=True)
@click.option('--extract-timeout-minutes', type=int, default=5)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would set the default at least to 10 minutes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See next push.

# @click.option('--extract-check-interval-seconds', type=int, default=10)
@click.option('--display-progress/--no-display-progress', type=bool, default=True)
def language_container_deployer_main(
bucketfs_name: str,
bucketfs_host: str,
Expand Down Expand Up @@ -179,6 +183,8 @@ def language_container_deployer_main(
alter_system: bool,
allow_override: bool,
wait_for_completion: bool,
extract_timeout_minutes: int,
display_progress: bool,
container_url: Optional[str] = None,
container_name: Optional[str] = None):

Expand All @@ -203,7 +209,10 @@ def language_container_deployer_main(
ssl_trusted_ca=ssl_cert_path,
ssl_client_certificate=ssl_client_cert_path,
ssl_private_key=ssl_client_private_key,
use_ssl_cert_validation=use_ssl_cert_validation)
use_ssl_cert_validation=use_ssl_cert_validation,
extract_timeout=timedelta(minutes=extract_timeout_minutes),
display_progress=display_progress,
)

if not upload_container:
deployer.run(alter_system=alter_system, allow_override=allow_override,
Expand Down
19 changes: 10 additions & 9 deletions test/unit/deployment/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
SecretParams,
)
from exasol.python_extension_common.deployment.language_container_deployer import LanguageContainerDeployer
from datetime import timedelta


class CliRunner:
Expand Down Expand Up @@ -62,13 +63,15 @@ def __init__(
self,
api: str,
value: any = None,
cli_value: any = None,
cli: str = None,
env: str = None,
prompt: str = None,
):
self.api_kwarg = api
self.value = f"om_value_{api}" if value is None else value
self.cli = cli or "--" + api.replace("_", "-")
self.cli_value = self.value if cli_value is None else cli_value
self.env = env
self.prompt = prompt

Expand Down Expand Up @@ -144,6 +147,9 @@ def test_default_values(container_file):
OptionMapper("ssl_client_certificate", "", cli="--ssl-client-cert-path"),
OptionMapper("ssl_private_key", "", cli="--ssl-client-private-key"),
OptionMapper("use_ssl_cert_validation", True),
OptionMapper("extract_timeout", cli="--extract-timeout-minutes",
cli_value=5, value=timedelta(minutes=5)),
OptionMapper("display_progress", True),
]
deployer = create_autospec(LanguageContainerDeployer)
with CliRunner(deployer) as runner:
Expand Down Expand Up @@ -187,19 +193,17 @@ def test_cli_options_passed_to_create(container_file):
OptionMapper("ssl_client_certificate", cli="--ssl-client-cert-path"),
OptionMapper("ssl_private_key", cli="--ssl-client-private-key"),
OptionMapper("use_ssl_cert_validation", False),
# For the following two arguments to
# LanguageContainerDeployer.create() there are no corresponding CLI
# options defined:
# - container_url: Optional[str] = None,
# - container_name: Optional[str] = None):
OptionMapper("extract_timeout", cli="--extract-timeout-minutes",
cli_value=6, value=timedelta(minutes=6)),
OptionMapper("display_progress", False),
]
def keys_and_values():
for o in options:
if o.value == False:
yield "--no-" + o.cli[2:]
continue
yield o.cli
yield str(o.value)
yield str(o.cli_value)

cli_options = list(keys_and_values())
deployer = create_autospec(LanguageContainerDeployer)
Expand Down Expand Up @@ -261,6 +265,3 @@ def test_container_url():

Hence this test case currently cannot be run.
"""

# Additionally there seems to be a file main.py missing that is wrapping the
# command line.
Loading