Skip to content

Commit

Permalink
Merge pull request #1436 from Sage-Bionetworks/develop-fds-1956
Browse files Browse the repository at this point in the history
technical debt: Remove service_acct_creds_synapse_id from schematic config
  • Loading branch information
linglp authored Jun 10, 2024
2 parents 570ee62 + c33d1bb commit 514f65e
Show file tree
Hide file tree
Showing 9 changed files with 0 additions and 64 deletions.
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,6 @@ model:
# This section is for using google sheets with Schematic
google_sheets:
# The Synapse id of the Google service account credentials.
service_acct_creds_synapse_id: "syn25171627"
# Path to the synapse config file, either absolute or relative to this file
service_acct_creds: "schematic_service_account_creds.json"
# When doing google sheet validation (regex match) with the validation rules.
Expand Down
2 changes: 0 additions & 2 deletions config_example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ model:

# This section is for using google sheets with Schematic
google_sheets:
# The Synapse id of the Google service account credentials.
service_acct_creds_synapse_id: "syn25171627"
# Path to the synapse config file, either absolute or relative to this file
service_acct_creds: "schematic_service_account_creds.json"
# When doing google sheet validation (regex match) with the validation rules.
Expand Down
8 changes: 0 additions & 8 deletions schematic/configuration/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,6 @@ def model_location(self) -> str:
"""
return self._model_config.location

@property
def service_account_credentials_synapse_id(self) -> str:
"""
Returns:
str: The Synapse id of the Google service account credentials.
"""
return self._google_sheets_config.service_acct_creds_synapse_id

@property
def service_account_credentials_path(self) -> str:
"""
Expand Down
3 changes: 0 additions & 3 deletions schematic/configuration/dataclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,10 @@ class GoogleSheetsConfig:
strict_validation: When doing google sheet validation (regex match) with the validation rules.
True is alerting the user and not allowing entry of bad values.
False is warning but allowing the entry on to the sheet.
service_acct_creds_synapse_id: The Synapse id of the Google service account credentials.
service_acct_creds: Path to the Google service account credentials,
either absolute or relative to this file
"""

service_acct_creds_synapse_id: str = "syn25171627"
service_acct_creds: str = "schematic_service_account_creds.json"
strict_validation: bool = True

Expand All @@ -151,7 +149,6 @@ def validate_string_is_not_empty(cls, value: str) -> str:
raise ValueError(f"{value} is an empty string")
return value

@validator("service_acct_creds_synapse_id")
@classmethod
def validate_synapse_id(cls, value: str) -> str:
"""Check if string is a valid synapse id
Expand Down
34 changes: 0 additions & 34 deletions schematic/utils/google_api_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from googleapiclient.discovery import build, Resource # type: ignore
from google.oauth2 import service_account # type: ignore
from schematic.configuration.configuration import CONFIG
from schematic.store.synapse import SynapseStorage

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -67,39 +66,6 @@ def build_service_account_creds() -> GoogleServiceAcountCreds:
return creds


def download_creds_file() -> None:
"""Download google credentials file"""
syn = SynapseStorage.login()

# if file path of service_account does not exist
# and if an environment variable related to service account is not found
# regenerate service_account credentials
if (
not os.path.exists(CONFIG.service_account_credentials_path)
and "SERVICE_ACCOUNT_CREDS" not in os.environ
):
# synapse ID of the 'schematic_service_account_creds.json' file
api_creds = CONFIG.service_account_credentials_synapse_id

# Download in parent directory of SERVICE_ACCT_CREDS to
# ensure same file system for os.rename()
creds_dir = os.path.dirname(CONFIG.service_account_credentials_path)

creds_file = syn.get(api_creds, downloadLocation=creds_dir)
os.rename(creds_file.path, CONFIG.service_account_credentials_path)

logger.info(
"The credentials file has been downloaded "
f"to '{CONFIG.service_account_credentials_path}'"
)

elif "SERVICE_ACCOUNT_CREDS" in os.environ:
# remind users that "SERVICE_ACCOUNT_CREDS" as an environment variable is being used
logger.info(
"Using environment variable SERVICE_ACCOUNT_CREDS as the credential file."
)


@no_type_check
def execute_google_api_requests(service, requests_body, **kwargs) -> Any:
"""
Expand Down
1 change: 0 additions & 1 deletion tests/data/test_configs/default_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,5 @@ model:
location: 'tests/data/example.model.jsonld'

google_sheets:
service_acct_creds_synapse_id: 'syn25171627'
service_acct_creds: "schematic_service_account_creds.json"
strict_validation: true
1 change: 0 additions & 1 deletion tests/data/test_configs/valid_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,5 @@ model:
location: "model.jsonld"

google_sheets:
service_acct_creds_synapse_id: "syn1"
service_acct_creds: "creds.json"
strict_validation: false
1 change: 0 additions & 1 deletion tests/data/test_configs/valid_config2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@ model:
location: "model.jsonld"

google_sheets:
service_acct_creds_synapse_id: "syn1"
service_acct_creds: "creds.json"
strict_validation: false
12 changes: 0 additions & 12 deletions tests/test_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,27 +80,18 @@ def test_google_sheets_config(self) -> None:
assert isinstance(
GoogleSheetsConfig(
service_acct_creds="file_name",
service_acct_creds_synapse_id="syn1",
strict_validation=True,
),
GoogleSheetsConfig,
)
with pytest.raises(ValidationError):
GoogleSheetsConfig(
service_acct_creds="file_name",
service_acct_creds_synapse_id="syn1",
strict_validation="tru",
)
with pytest.raises(ValidationError):
GoogleSheetsConfig(
service_acct_creds="",
service_acct_creds_synapse_id="syn1",
strict_validation=True,
)
with pytest.raises(ValidationError):
GoogleSheetsConfig(
service_acct_creds="file_name",
service_acct_creds_synapse_id="syn",
strict_validation=True,
)

Expand All @@ -120,7 +111,6 @@ def test_init(self) -> None:
assert config.manifest_title == "example"
assert config.manifest_data_type == ["Biospecimen", "Patient"]
assert config.model_location == "tests/data/example.model.jsonld"
assert config.service_account_credentials_synapse_id
assert (
config.service_account_credentials_path
!= "schematic_service_account_creds.json"
Expand Down Expand Up @@ -158,7 +148,6 @@ def test_load_config1(self) -> None:
assert config.manifest_title == "example"
assert config.manifest_data_type == ["Biospecimen", "Patient"]
assert config.model_location == "tests/data/example.model.jsonld"
assert config.service_account_credentials_synapse_id
assert (
config.service_account_credentials_path
!= "schematic_service_account_creds.json"
Expand Down Expand Up @@ -188,7 +177,6 @@ def test_load_config2(self) -> None:
assert config.manifest_title == "title"
assert config.manifest_data_type == ["data_type"]
assert config.model_location == "model.jsonld"
assert config.service_account_credentials_synapse_id
assert os.path.basename(config.service_account_credentials_path) == "creds.json"
assert config.google_sheets_master_template_id == (
"1LYS5qE4nV9jzcYw5sXwCza25slDfRA1CIg3cs-hCdpU"
Expand Down

0 comments on commit 514f65e

Please sign in to comment.