Skip to content

Commit

Permalink
MLCOMPUTE-967 | Spark config calculation should result in failure if …
Browse files Browse the repository at this point in the history
…srv-configs can't be read (#142)

* MLCOMPUTE-967 | Spark config calculation should result in failure if srv-configs can't be read

* MLCOMPUTE-967 | throw exception instead of sys exit

* MLCOMPUTE-967 | add warning log

* MLCOMPUTE-967 | bump version

---------

Co-authored-by: Sameer Sharma <sameersharma@yelp.com>
  • Loading branch information
CaptainSame and Sameer Sharma committed Feb 28, 2024
1 parent d6f9e65 commit e427503
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 15 deletions.
28 changes: 17 additions & 11 deletions service_configuration_lib/spark_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ def _load_aws_credentials_from_yaml(yaml_file_path) -> Tuple[str, str, Optional[

def get_aws_credentials(
service: Optional[str] = DEFAULT_SPARK_SERVICE,
no_aws_credentials: bool = False,
aws_credentials_yaml: Optional[str] = None,
profile_name: Optional[str] = None,
session: Optional[boto3.Session] = None,
Expand All @@ -114,9 +113,7 @@ def get_aws_credentials(
assume_role_user_creds_file: str = '/nail/etc/spark_role_assumer/spark_role_assumer.yaml',
) -> Tuple[Optional[str], Optional[str], Optional[str]]:
"""load aws creds using different method/file"""
if no_aws_credentials:
return None, None, None
elif aws_credentials_yaml:
if aws_credentials_yaml:
return _load_aws_credentials_from_yaml(aws_credentials_yaml)
elif aws_credentials_json:
with open(aws_credentials_json, 'r') as f:
Expand Down Expand Up @@ -397,6 +394,8 @@ def __init__(self):
) = utils.load_spark_srv_conf()
except Exception as e:
log.error(f'Failed to load Spark srv configs: {e}')
# should fail because Spark config calculation depends on values in srv-configs
raise e

def _append_spark_prometheus_conf(self, spark_opts: Dict[str, str]) -> Dict[str, str]:
spark_opts['spark.ui.prometheus.enabled'] = 'true'
Expand Down Expand Up @@ -1067,13 +1066,20 @@ def get_spark_conf(
# Pick a port from a pre-defined port range, which will then be used by our Jupyter
# server metric aggregator API. The aggregator API collects Prometheus metrics from multiple
# Spark sessions and exposes them through a single endpoint.
ui_port = int(
(spark_opts_from_env or {}).get('spark.ui.port') or
utils.ephemeral_port_reserve_range(
self.spark_constants.get('preferred_spark_ui_port_start'),
self.spark_constants.get('preferred_spark_ui_port_end'),
),
)
try:
ui_port = int(
(spark_opts_from_env or {}).get('spark.ui.port') or
utils.ephemeral_port_reserve_range(
self.spark_constants.get('preferred_spark_ui_port_start'),
self.spark_constants.get('preferred_spark_ui_port_end'),
),
)
except Exception as e:
log.warning(
f'Could not get an available port using srv-config port range: {e}. '
'Using default port range to get an available port.',
)
ui_port = utils.ephemeral_port_reserve_range()

spark_conf = {**(spark_opts_from_env or {}), **_filter_user_spark_opts(user_spark_opts)}
random_postfix = utils.get_random_string(4)
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

setup(
name='service-configuration-lib',
version='2.18.14',
version='2.18.15',
provides=['service_configuration_lib'],
description='Start, stop, and inspect Yelp SOA services',
url='https://github.com/Yelp/service_configuration_lib',
Expand Down
3 changes: 0 additions & 3 deletions tests/spark_config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ class TestGetAWSCredentials:
creds = {'aws_access_key_id': access_key, 'aws_secret_access_key': secret_key}
expected_creds = (access_key, secret_key, None)

def test_no_aws_creds(self):
assert spark_config.get_aws_credentials(no_aws_credentials=True) == (None, None, None)

@pytest.mark.parametrize('aws_creds', [temp_creds, creds])
def test_aws_credentials_yaml(self, tmpdir, aws_creds):
fp = tmpdir.join('test.yaml')
Expand Down

0 comments on commit e427503

Please sign in to comment.