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

Repair OLIAPI #1512

Open
wants to merge 37 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
bd2caa8
checkpoint: add scaling to cstr but didn't note any affect on BSM2-P …
adam-a-a Oct 7, 2024
05b717a
Merge branch 'main' of https://github.com/watertap-org/watertap
adam-a-a Oct 18, 2024
35c42dc
fix base engine url to mitigate bad request error
adam-a-a Oct 18, 2024
e9ad119
added this to mitigate error that arose while trying composition surv…
adam-a-a Oct 18, 2024
556f679
Merge branch 'main' of https://github.com/watertap-org/watertap
adam-a-a Oct 29, 2024
18a708f
Merge branch 'main' of https://github.com/watertap-org/watertap
adam-a-a Oct 30, 2024
5cf8414
Merge branch 'main' into repair_oliapi_oct24
adam-a-a Oct 30, 2024
6a2d5c9
Revert "checkpoint: add scaling to cstr but didn't note any affect on…
adam-a-a Oct 30, 2024
a6f6d43
blk
adam-a-a Oct 30, 2024
37e9a7a
update logger messages to be more clear on uploading/generating/delet…
adam-a-a Oct 31, 2024
b635751
remove import
adam-a-a Oct 31, 2024
71cc4e7
try inspecting notebook
adam-a-a Oct 31, 2024
5aa1e95
attempt to resolve notebook error, clear up logger messages
adam-a-a Oct 31, 2024
7b5288b
temporarily comment out failing survey tests
adam-a-a Oct 31, 2024
116fcdb
unclear why survey tests fail on GH when they pass locally with same …
adam-a-a Oct 31, 2024
8041b20
temporarily comment out dbs file cleanup upon exit
adam-a-a Nov 1, 2024
7deba9a
bring back session_dbs_files deletion
adam-a-a Nov 1, 2024
8acadda
update tests with suspected fix regarding dbs file usage and also try…
adam-a-a Nov 1, 2024
ea309a1
revise tests
adam-a-a Nov 1, 2024
59766c3
tests passed after pushing last commit! let's see if reverting this c…
adam-a-a Nov 1, 2024
24485e7
bring back refresh since it seems that wasn't the solution (alone) an…
adam-a-a Nov 1, 2024
363d652
undoing more changes to see which lines resolved the issue
adam-a-a Nov 1, 2024
4a378e8
bringing back changes that I suspect were the true culprit before
adam-a-a Nov 1, 2024
a9b9fe6
set refresh False again
adam-a-a Nov 1, 2024
8ca5f21
undoing more to identify issue
adam-a-a Nov 1, 2024
a5a7215
seems that tests fail when refresh=False and conftest changes are rev…
adam-a-a Nov 1, 2024
dd88d62
bring back all changes that led to success
adam-a-a Nov 1, 2024
0e10833
undo change to exception message
adam-a-a Nov 1, 2024
faf0d10
run black
adam-a-a Nov 1, 2024
e28de47
move refresh attribute earlier; troubleshoot issue with unexpected re…
adam-a-a Nov 5, 2024
d2af67b
blk
adam-a-a Nov 7, 2024
6d9241b
try/except for JSONDecodeError and retry after 1s delay; only prompt …
adam-a-a Nov 27, 2024
9a125aa
Merge branch 'main' into repair_oliapi_oct24
adam-a-a Nov 27, 2024
4db8d8d
black
adam-a-a Nov 27, 2024
18b9b04
Merge branch 'repair_oliapi_oct24' of https://github.com/adam-a-a/wat…
adam-a-a Nov 27, 2024
0c6d888
add debug_level to credentialmanager; turn interactive mdoe off in tu…
adam-a-a Nov 27, 2024
67819fa
black
adam-a-a Nov 27, 2024
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
12 changes: 7 additions & 5 deletions tutorials/oli_calculations.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@
"credential_manager = CredentialManager(\n",
" access_keys=[os.environ[\"OLI_API_KEY\"]],\n",
" root_url=os.environ[\"OLI_API_ROOT_URL\"],\n",
" interactive_mode=True,\n",
" interactive_mode=False,\n",
" test=True,\n",
")\n",
"'''\n",
Expand Down Expand Up @@ -411,16 +411,18 @@
" dbs_files = oliapi.get_user_dbs_file_ids()\n",
" \n",
" # uncomment the following lines to view results:\n",
" #for idx, file in enumerate(dbs_files):\n",
" # print(f\"{idx+1}\\t{file}\")\n",
" for idx, file in enumerate(dbs_files):\n",
" print(f\"{idx+1}\\t{file}\")\n",
" \n",
" # if a DBS file has been retained from a previous session,\n",
" # its flash history and chemistry information can be summarized.\n",
" file_summary = oliapi.get_dbs_file_summary(dbs_file_id)\n",
"\n",
" print(file_summary)\n",
" \n",
" # save chemistry information\n",
" chemistry_info = file_summary[\"chemistry_info\"]\n",
" write_output(chemistry_info[\"result\"], \"chemistry_info.json\")\n",
" # Uncomment next line to save as json file\n",
" # write_output(chemistry_info[\"result\"], \"chemistry_info.json\")\n",
" \n",
" # DBS files can also be deleted.\n",
" oliapi.dbs_file_cleanup(dbs_files)\n",
Expand Down
65 changes: 49 additions & 16 deletions watertap/tools/oli_api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@
import sys
import json
import time

from pyomo.common.dependencies import attempt_import

requests, requests_available = attempt_import("requests", defer_check=False)
from requests import JSONDecodeError

from watertap.tools.oli_api.util.watertap_to_oli_helper_functions import get_oli_name

Expand Down Expand Up @@ -95,6 +95,9 @@
else:
_logger.setLevel(logging.DEBUG)

# TODO: unknown bug where only "liquid1" phase is found in Flash analysis
self.valid_phases = ["liquid1", "vapor", "solid", "liquid2"]

# binds OLIApi instance to context manager
def __enter__(self):
self.session_dbs_files = []
Expand All @@ -103,13 +106,18 @@
# return False if no exceptions raised
def __exit__(self, exc_type=None, exc_value=None, traceback=None):
# delete all .dbs files created during session
_logger.info(
f"Exiting: deleting {len(self.session_dbs_files)} remaining DBS files created during the session that were not marked by keep_file=True."
)
self.dbs_file_cleanup(self.session_dbs_files)
return False

def _prompt(self, msg, default=""):
if self.interactive_mode:
msg = msg + "Enter [y]/n to proceed."

Check warning on line 117 in watertap/tools/oli_api/client.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/client.py#L117

Added line #L117 was not covered by tests
return input(msg)
Comment on lines 116 to 118
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the interactive mode functionality work in Jupyter? It doesn't seem to be the case, and, if so, perhaps it should be set to False in the tutorial

else:
msg = msg + "To choose [y]/n to this action, set interactive_mode=True."
_logger.info(msg)
return default

Expand All @@ -133,7 +141,7 @@
if bool(dbs_file_id):
if not keep_file:
self.session_dbs_files.append(dbs_file_id)
_logger.info(f"DBS file ID is {dbs_file_id}")
_logger.info(f"Uploaded DBS file ID is {dbs_file_id}")
return dbs_file_id
else:
raise RuntimeError("Unexpected failure getting DBS file ID.")
Expand Down Expand Up @@ -191,8 +199,6 @@
else:
dbs_file_inputs["modelName"] = "OLI_analysis"

# TODO: unknown bug where only "liquid1" phase is found in Flash analysis
self.valid_phases = ["liquid1", "vapor", "solid", "liquid2"]
if phases is not None:
invalid_phases = [p for p in phases if p not in self.valid_phases]
if invalid_phases:
Expand Down Expand Up @@ -230,7 +236,7 @@
if bool(dbs_file_id):
if not keep_file:
self.session_dbs_files.append(dbs_file_id)
_logger.info(f"DBS file ID is {dbs_file_id}")
_logger.info(f"Generated DBS file ID is {dbs_file_id}")
return dbs_file_id
else:
raise RuntimeError("Unexpected failure getting DBS file ID.")
Expand Down Expand Up @@ -289,10 +295,15 @@
"""

if dbs_file_ids is None:
_logger.info(

Check warning on line 298 in watertap/tools/oli_api/client.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/client.py#L298

Added line #L298 was not covered by tests
"No DBS file IDs were provided to the dbs_file_cleanup method. Checking user's cloud account for DBS file IDs."
)
dbs_file_ids = self.get_user_dbs_file_ids()
r = self._prompt(
f"WaterTAP will delete {len(dbs_file_ids)} DBS files [y]/n ", "y"
)
if not len(dbs_file_ids):
_logger.info("No DBS file IDs were found on the user's cloud account.")
return

Check warning on line 304 in watertap/tools/oli_api/client.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/client.py#L302-L304

Added lines #L302 - L304 were not covered by tests

r = self._prompt(f"WaterTAP will delete {len(dbs_file_ids)} DBS files. ", "y")
if (r.lower() == "y") or (r == ""):
for dbs_file_id in dbs_file_ids:
_logger.info(f"Deleting {dbs_file_id} ...")
Expand All @@ -302,8 +313,16 @@
headers=self.credential_manager.headers,
)
req = _request_status_test(req, ["SUCCESS"])

if req["status"] == "SUCCESS":
_logger.info(f"File deleted")
# Remove the file from session_dbs_files list if it is there, otherwise an error will occur upon exit when this method is called again and already deleted files will remain on the list for deletion. Thus, an error can occur if there is no existing ID to delete.
if dbs_file_id in self.session_dbs_files:
self.session_dbs_files.remove(dbs_file_id)
_logger.info(
f"File {dbs_file_id} deleted and removed from session_dbs_files list."
)
else:
_logger.info(f"File {dbs_file_id} deleted.")

Check warning on line 325 in watertap/tools/oli_api/client.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/client.py#L325

Added line #L325 was not covered by tests

def get_corrosion_contact_surfaces(self, dbs_file_id):
"""
Expand Down Expand Up @@ -428,10 +447,21 @@
"""

mode, url, headers = self._get_flash_mode(dbs_file_id, flash_method)
req = requests.request(
mode, url, headers=headers, data=json.dumps(input_params)
)
req_json = _request_status_test(req, ["SUCCESS"])
try:
req = requests.request(
mode, url, headers=headers, data=json.dumps(input_params)
)
req_json = _request_status_test(req, ["SUCCESS"])
except JSONDecodeError:
delay = 1
time.sleep(delay)
_logger.debug(

Check warning on line 458 in watertap/tools/oli_api/client.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/client.py#L455-L458

Added lines #L455 - L458 were not covered by tests
f"JSONDecodeError occurred. Trying to convert response object to JSON again."
)
req = requests.request(

Check warning on line 461 in watertap/tools/oli_api/client.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/client.py#L461

Added line #L461 was not covered by tests
mode, url, headers=headers, data=json.dumps(input_params)
)
req_json = _request_status_test(req, ["SUCCESS"])

Check warning on line 464 in watertap/tools/oli_api/client.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/client.py#L464

Added line #L464 was not covered by tests
result_link = _get_result_link(req_json)
result = _poll_result_link(result_link, headers, max_request, poll_time)
return result
Expand All @@ -454,7 +484,7 @@
if "resultsLink" in req_json["data"]:
result_link = req_json["data"]["resultsLink"]
if not result_link:
raise RuntimeError(f"Failed to get 'resultsLink'. Response: {req.json()}")
raise RuntimeError(f"Failed to get 'resultsLink'. Response: {req_json.json()}")

Check warning on line 487 in watertap/tools/oli_api/client.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/client.py#L487

Added line #L487 was not covered by tests
return result_link


Expand All @@ -467,18 +497,21 @@

:return req_json: response object converted to JSON
"""

req_json = req.json()

func_name = sys._getframe().f_back.f_code.co_name
_logger.debug(f"{func_name} response: {req_json}")

if req.status_code == 200:
if target_keys:
if "status" in req_json:
if req_json["status"] in target_keys:
return req_json
else:
return req_json
raise RuntimeError(f"Failure in {func_name}. Response: {req_json}")
raise RuntimeError(

Check warning on line 512 in watertap/tools/oli_api/client.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/client.py#L512

Added line #L512 was not covered by tests
f"Failure in {func_name}. Response: {req_json}. Status Code: {req.status_code}."
)


def _poll_result_link(result_link, headers, max_request, poll_time):
Expand Down
6 changes: 2 additions & 4 deletions watertap/tools/oli_api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ def auth_credentials() -> dict:
def oliapi_instance(
tmp_path: Path,
auth_credentials: dict,
local_dbs_file: Path,
source_water: dict,
) -> OLIApi:

if not cryptography_available:
Expand All @@ -93,11 +91,11 @@ def oliapi_instance(
credentials = {
**auth_credentials,
"config_file": cred_file_path,
"refresh": True,
"interactive_mode": False,
}
credential_manager = CredentialManager(**credentials, test=True)
with OLIApi(credential_manager, interactive_mode=False) as oliapi:
oliapi.upload_dbs_file(str(local_dbs_file))
oliapi.generate_dbs_file(source_water)
yield oliapi
with contextlib.suppress(FileNotFoundError):
cred_file_path.unlink()
Expand Down
60 changes: 38 additions & 22 deletions watertap/tools/oli_api/credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
_logger.setLevel(logging.DEBUG)


# TODO: consider letting user set logging level instead of using interactive mode
class CredentialManager:
"""
A class to handle credentials for OLI Cloud.
Expand All @@ -89,6 +88,8 @@
access_keys=[],
test=False,
interactive_mode=True,
refresh=False,
debug_level="INFO",
):
"""
Manages credentials for OLIApi authentication requests.
Expand All @@ -101,24 +102,30 @@
:param encryption_key: fernet key generated by credential manager object
:param access_keys: list of access keys generated by user
:param test: bool switch for automation during tests
:param interactive_mode: bool to switch level of logging display from info to debug only
:param interactive_mode: enables direct interaction with user through prompts
:param refresh: bool to use refresh token in login method
MarcusHolly marked this conversation as resolved.
Show resolved Hide resolved
:param debug_level: string defining level of logging activity

"""

self.test = test
self.access_key = ""
self.encryption_key = encryption_key
self.config_file = Path(config_file).resolve()
self.refresh = refresh
self.interactive_mode = interactive_mode
self.debug_level = debug_level
if self.debug_level == "INFO":
_logger.setLevel(logging.INFO)
else:
_logger.setLevel(logging.DEBUG)

Check warning on line 121 in watertap/tools/oli_api/credentials.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/credentials.py#L121

Added line #L121 was not covered by tests
self._manage_credentials(
username,
password,
root_url,
auth_url,
access_keys,
)
if interactive_mode:
_logger.setLevel(logging.INFO)
else:
_logger.setLevel(logging.DEBUG)
self.set_headers()

def set_headers(self):
Expand Down Expand Up @@ -191,7 +198,7 @@
self.dbs_url = self.credentials["root_url"] + "/channel/dbs"
self.upload_dbs_url = self.credentials["root_url"] + "/channel/upload/dbs"
self.access_key_url = self.credentials["root_url"] + "/user/api-key"
self.engine_url = self.credentials["root_url"] + "/engine/"
self.engine_url = self.credentials["root_url"] + "/engine"
self._delete_dbs_url = self.credentials["root_url"] + "/channel/file/"

def _decrypt_credentials(self):
Expand Down Expand Up @@ -233,12 +240,13 @@
if self.test:
return True
else:
r = input(
"WaterTAP will write encrypted file to store OLI Cloud credentials: [y]/n: "
)
if (r.lower() == "y") or (r == ""):
return True
return False
if self.interactive_mode:
r = input(

Check warning on line 244 in watertap/tools/oli_api/credentials.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/credentials.py#L243-L244

Added lines #L243 - L244 were not covered by tests
"WaterTAP will write encrypted file to store OLI Cloud credentials: [y]/n: "
)
if (r.lower() == "y") or (r == ""):
return True
return False

Check warning on line 249 in watertap/tools/oli_api/credentials.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/credentials.py#L247-L249

Added lines #L247 - L249 were not covered by tests

# TODO: consider updating credentials when writing, rather than resetting blank ones
def _encrypt_credentials(self):
Expand Down Expand Up @@ -336,12 +344,10 @@
_logger.info(response.text)
return response.text

def login(self, refresh=False):
def login(self):
"""
Log in to OLI Cloud using access key or credentials.

:param refresh: bool to get refresh token

:return status: bool indicating success or failure
"""

Expand All @@ -356,12 +362,22 @@
)
else:
_logger.info("Logging into OLI API using username and password")
if refresh:
body = {
"refresh_token": self.refresh_token,
"grant_type": "refresh_token",
"client_id": "apiclient",
}
if self.refresh:
if hasattr(self, "refresh_token"):
body = {

Check warning on line 367 in watertap/tools/oli_api/credentials.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/credentials.py#L365-L367

Added lines #L365 - L367 were not covered by tests
"refresh_token": self.refresh_token,
"grant_type": "refresh_token",
"client_id": "apiclient",
}
else:
body = {

Check warning on line 373 in watertap/tools/oli_api/credentials.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/credentials.py#L373

Added line #L373 was not covered by tests
"username": self.credentials["username"],
"password": self.credentials["password"],
"grant_type": "password",
"client_id": "apiclient",
}
status = self.auth_status(body, req_result)
return status

Check warning on line 380 in watertap/tools/oli_api/credentials.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/credentials.py#L379-L380

Added lines #L379 - L380 were not covered by tests
else:
body = {
"username": self.credentials["username"],
Expand Down
8 changes: 5 additions & 3 deletions watertap/tools/oli_api/flash.py
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,9 @@
pass
elif k in d["inflows"]["values"]:
d = d["inflows"]["values"]
elif k in d["corrosionParameters"]:
elif hasattr(d, "corrosionParameters") and (

Check warning on line 975 in watertap/tools/oli_api/flash.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/flash.py#L975

Added line #L975 was not covered by tests
k in d["corrosionParameters"]
):
d = d["corrosionParameters"]
else:
_logger.warning(f"Survey key {k} not found in JSON input.")
Expand Down Expand Up @@ -1158,9 +1160,9 @@
prop_tag = _get_nested_data(result, prop)["name"]
else:
_logger.warning(
f"Unexpected result:\n{result}\n\ninput_dict:\n{input_dict}"
f"Unexpected result:\n{result}\n\ninput_dict:\n{input_dict} from prop {prop}"
)

continue

Check warning on line 1165 in watertap/tools/oli_api/flash.py

View check run for this annotation

Codecov / codecov/patch

watertap/tools/oli_api/flash.py#L1165

Added line #L1165 was not covered by tests
label = f"{prop_tag}_{phase_tag}" if phase_tag else prop_tag
input_dict[k][label] = _extract_values(result, prop)
return input_dict
Expand Down
18 changes: 18 additions & 0 deletions watertap/tools/oli_api/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,26 @@ def test_dbs_file_available_for_testing(local_dbs_file: Path):
assert local_dbs_file.is_file()


@pytest.mark.unit
def test_generate_dbs_file(
oliapi_instance: OLIApi, local_dbs_file: Path, source_water: dict
):
dbs_file_id = oliapi_instance.generate_dbs_file(source_water)
assert len(dbs_file_id) > 0


@pytest.mark.unit
def test_upload_dbs_file(
oliapi_instance: OLIApi, local_dbs_file: Path, source_water: dict
):
dbs_file_id = oliapi_instance.upload_dbs_file(str(local_dbs_file))
assert len(dbs_file_id) > 0


@pytest.mark.unit
def test_dbs_file_cleanup(oliapi_instance: OLIApi, local_dbs_file: Path):
# This test checks both the upload_dbs_file method and dbs_file_cleanup method
# The following line will return 3 DBS file IDs. Note, the same file is uploaded three times, but each time a new ID is assigned.
ids = [oliapi_instance.upload_dbs_file(str(local_dbs_file)) for i in range(3)]
oliapi_instance.dbs_file_cleanup(ids)

Expand Down
Loading
Loading