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 all 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
9 changes: 5 additions & 4 deletions doc/changes/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

## Features

* #79 Added function get_cli_arg that makes a string CLI argument from an option and its value.
Also allowed passing an option name instead of the StdParams in the following two functions:
create_std_option, check_params.
* #52: Added timeout options for SLC deployer to CLI
* #79 Added function `get_cli_arg` that makes a string CLI argument from an option and its value.
Also allowed passing an option name instead of the `StdParams` in the following two functions:
`create_std_option`, `check_params`.

## Bug fixing

* #78 Missing default value in the definition of StdParams.path_in_bucket.
* #78 Missing default value in the definition of `StdParams.path_in_bucket`.
6 changes: 5 additions & 1 deletion exasol/python_extension_common/cli/std_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ class StdParams(Enum):
alter_system = (StdTags.SLC, auto())
allow_override = (StdTags.SLC, auto())
wait_for_completion = (StdTags.SLC, auto())
deploy_timeout_minutes = (StdTags.SLC, auto())
display_progress = (StdTags.SLC, auto())

def __init__(self, tags: StdTags, value):
self.tags = tags
Expand Down Expand Up @@ -157,7 +159,9 @@ def _get_param_name(std_param: StdParamOrName) -> str:
StdParams.upload_container: {'type': bool, 'default': True},
StdParams.alter_system: {'type': bool, 'default': True},
StdParams.allow_override: {'type': bool, 'default': False},
StdParams.wait_for_completion: {'type': bool, 'default': True}
StdParams.wait_for_completion: {'type': bool, 'default': True},
StdParams.deploy_timeout_minutes: {'type': int, 'default': 10},
StdParams.display_progress: {'type': bool, 'default': True},
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class ExtractValidator:
def __init__(self,
pyexasol_connection: pyexasol.ExaConnection,
timeout: timedelta,
interval: timedelta = timedelta(seconds=10),
interval: timedelta = timedelta(seconds=30),
callback: Callable[[int, List[int]], None] | None = None,
) -> None:
self._pyexasol_conn = pyexasol_connection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,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 All @@ -105,8 +109,8 @@ def __init__(self,
else:
self._extract_validator = ExtractValidator(
pyexasol_connection,
timeout=timedelta(minutes=5),
interval=timedelta(seconds=10),
timeout=timedelta(minutes=10),
interval=timedelta(seconds=30),
)
logger.debug("Init %s", LanguageContainerDeployer.__name__)

Expand Down Expand Up @@ -330,7 +334,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,
deploy_timeout: timedelta = timedelta(minutes=10),
display_progress: bool = False,
) -> "LanguageContainerDeployer":
warnings.warn(
"create() function is deprecated and will be removed in a future version. "
"For CLI use the LanguageContainerDeployerCli class instead.",
Expand Down Expand Up @@ -389,4 +396,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, deploy_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 @@ -6,6 +6,7 @@
import click
import warnings
from exasol.python_extension_common.deployment.language_container_deployer import LanguageContainerDeployer
from datetime import timedelta


class CustomizableParameters(Enum):
Expand Down Expand Up @@ -154,6 +155,8 @@ 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('--deploy-timeout-minutes', 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 @@ -182,6 +185,8 @@ def language_container_deployer_main(
alter_system: bool,
allow_override: bool,
wait_for_completion: bool,
deploy_timeout_minutes: int,
display_progress: bool,
container_url: Optional[str] = None,
container_name: Optional[str] = None):
warnings.warn(
Expand Down Expand Up @@ -213,7 +218,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,
deploy_timeout=timedelta(minutes=deploy_timeout_minutes),
display_progress=display_progress,
)

if not upload_container:
deployer.run(alter_system=alter_system, allow_override=allow_override,
Expand Down
26 changes: 14 additions & 12 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("deploy_timeout", cli="--deploy-timeout-minutes",
cli_value=5, value=timedelta(minutes=10)),
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("deploy_timeout", cli="--deploy-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 @@ -250,17 +254,15 @@ def test_container_file():
"Covered by test_default_values()"


@pytest.mark.skip(reason="Not implemented, yet")
@pytest.mark.skip(reason="Test case to be implemented in derived applications")
def test_container_url():
"""
For the following two arguments to LanguageContainerDeployer.create()
there are no corresponding CLI options defined:
the corresponding CLI options need to defined in actual CLI applications
using ``language_container_deployer_main()``

- container_url: Optional[str] = None,
- container_name: Optional[str] = None):

Hence this test case currently cannot be run.
Hence there is no related test case in the current file.
"""

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