Skip to content

Commit

Permalink
Remove aws profile parameter from storage manager schema (#353)
Browse files Browse the repository at this point in the history
* remove aws_profile parameter

* remove/update test

* fix reading of the env variable

* fix issue with type

---------

Co-authored-by: Matic Lubej <matic.lubej@sinergise.com>
  • Loading branch information
mlubej and Matic Lubej committed Aug 12, 2024
1 parent d2e66b8 commit efa842f
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 48 deletions.
15 changes: 5 additions & 10 deletions eogrow/core/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

from __future__ import annotations

from typing import Any, ClassVar, Dict, Literal, Optional
import os
from typing import Any, ClassVar, Dict, Literal

import fs
from pydantic import BaseSettings, Field
Expand All @@ -24,13 +25,6 @@ class Schema(ManagerSchema, BaseSettings):
"If on AWS, the path must be prefixed with s3://."
),
)
aws_profile: Optional[str] = Field(
env="AWS_PROFILE",
description=(
"The AWS profile with credentials needed to access the S3 buckets. In case the profile isn't specified"
" with a parameter it can be read from an environmental variable."
),
)
filesystem_kwargs: Dict[str, Any] = Field(
default_factory=dict, description="Optional kwargs to be passed on to FS specs."
)
Expand Down Expand Up @@ -66,8 +60,9 @@ def _prepare_sh_config(self) -> SHConfig:
will show a warning and return a config without AWS credentials."""
sh_config = SHConfig()

if self.is_on_s3() and self.config.aws_profile:
sh_config = get_aws_credentials(aws_profile=self.config.aws_profile, config=sh_config)
aws_profile = os.getenv("AWS_PROFILE")
if self.is_on_s3() and aws_profile is not None:
sh_config = get_aws_credentials(aws_profile=aws_profile, config=sh_config)

return sh_config

Expand Down
37 changes: 0 additions & 37 deletions tests/core/test_storage.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import os
from typing import Optional

import pytest
from botocore.exceptions import ProfileNotFound
from fs.osfs import OSFS
from fs_s3fs import S3FS

Expand Down Expand Up @@ -67,41 +65,6 @@ def test_get_custom_folder(local_storage_manager: StorageManager, project_folder
assert local_storage_manager.get_folder("eopatches", full_path=True) == abs_path


@pytest.mark.parametrize("config_profile", [None, "", "nonexistent-config-profile"])
@pytest.mark.parametrize("env_profile", [None, "", "nonexistent-env-profile"])
def test_aws_profile(aws_storage_config: RawConfig, config_profile: Optional[str], env_profile: Optional[str]):
"""Checks different combinations of profile being set with a config parameter and environmental variable. Checks
also that config parameter takes priority over environmental variable.
In the first step of this test, we add given profile name parameters into the config dictionary and into the
dictionary of environmental variables. Note that if profile name is `None`, we instead remove the parameter from a
dictionary altogether.
"""

for parameter_key, parameter_value, config_dict in [
("aws_profile", config_profile, aws_storage_config),
("AWS_PROFILE", env_profile, os.environ),
]:
if parameter_value is not None:
config_dict[parameter_key] = parameter_value
elif parameter_key in config_dict:
del config_dict[parameter_key]

try:
expected_profile = config_profile if config_profile is not None else env_profile
if expected_profile:
with pytest.raises(ProfileNotFound) as exception_info:
StorageManager.from_raw_config(aws_storage_config)

assert str(exception_info.value) == f"The config profile ({expected_profile}) could not be found"
else:
storage = StorageManager.from_raw_config(aws_storage_config)
assert storage.config.aws_profile == expected_profile
finally:
if "AWS_PROFILE" in os.environ:
del os.environ["AWS_PROFILE"]


@pytest.mark.parametrize(
"config",
[
Expand Down
1 change: 0 additions & 1 deletion tests/test_config_files/other/aws_storage_test.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"manager": "eogrow.core.storage.StorageManager",
"project_folder": "s3://eogrow-test-storage/test",
"aws_profile": null,
"structure": {
"batch": "tiffs",
"eopatches": "eopatches"
Expand Down

0 comments on commit efa842f

Please sign in to comment.