Add functionality for more descriptive console messages and return of response objects. #69

IanTBlack · 2025-11-10T21:28:07Z

Maybe a start for issue #68?

Adds slightly more descriptive and organized console messages. A keyword at class instantiation provides the options for removing a users token from output messages. There is also functionality for raising an HTTPError. Setting to false will return the full requests.Response object.

See https://github.com/IanTBlack/api-python-client/blob/main/scratch.ipynb for example.

I've only tested this on discovery and scalardata endpoints. If you'd like to implement the logger functionality, I can spend some time cleaning up the messages and test on other end points.

Thanks!

… requests objects.

kan-fu

Thanks for the PR! I invited you as a collaborator for this repo, so you can submit a PR from a branch in this repo instead of the fork/PR way if you would like.

There are some test failures and some documentation that need to be updated, which I can take care of.

Another thing is could you fix some styling issues by running black src;isort src and ruff check src (the commands can be found at tox.ini), and fix the merge conflict?

kan-fu · 2025-11-12T19:11:03Z

src/onc/modules/_Messages.py

+    return logger
+
+
+def scrub_token(input: str) -> str:


I believe it is better to use re.sub. It will not cause any exception if the pattern is valid, and the line 65 on _oncService.py could be simplified. For example,

import re re.sub(r'&token=[a-z0-9-]{8}', r'&token=REDACTED',"foo&token=foo-bar0&foo=bar")

BTW how did you figure out the token pattern is [a-f0-9-]? I always use [a-zA-Z0-9-] before without giving a second thought.

Couldn't tell you how I settled on that. I will change to your regex. Thanks.

I mean your regex works fine (the fact that token generation only uses a subset of alphanumeric is to my surprise). You don't need to change the regex, just change the function call to re.sub.

kan-fu · 2025-11-12T20:42:28Z

src/onc/modules/_Messages.py

+    :return: An error message.
+    """
+    payload = response.json()
+    if 'message' in payload.keys():


This if-else clause could be simplified by payload.get("message", None)

kan-fu · 2025-11-12T21:57:23Z

src/onc/modules/_Messages.py

+    Set up a logger object for displaying verbose messages to console.
+
+    :param logger_name: The unique logger name to use. Can be shared between modules
+    :param level: The logging level to use. Default is 2, which corresponds to DEBUG.


logging.DEBUG is 10, right? https://docs.python.org/3/howto/logging.html#logging-levels

Yes, that is an artifact from a separate project. Will change.

kan-fu · 2025-11-12T23:05:24Z

scratch.ipynb

Thanks for the illustration, but probably no need to be included in the PR. I guess it can be put in a temp branch.

kan-fu · 2025-11-13T19:44:31Z

src/onc/modules/_OncArchive.py

At line 172 there is also a mp = _MultiPage(self) that needs to be changed.

I am also thinking is it better to still keep the single parameter parent for the constructor of the _OncService and use the _config to access those parameters, like what Dany did for other class variables?

kan-fu · 2025-11-13T19:44:51Z

src/onc/onc.py

Let me just summarize my understanding about the logging features for the client library before this pr (I will just call Dany's code because he is the one who created the library).

So there are roughly three types of logs:

Logs that report the status of helper functions like orderDataProduct (Dany's log logic that in this module are not easy to follow IMO) and multi-page feature. In the code these are plain print statements, so they are more like interactive messages and cannot be turned off.

Logs that report the url or running time. These are controlled by the showInfo parameter. Default is False.

Logs that report warning messages. These are controlled by the showWarning parameter. Default is True (I changed it from False to True in my other PR. I will probably merge that first).

Besides the three kinds of logs, the exception messages about the errors also look like logs. These also cannot be suppressed and users are supposed to use try-except clause to bypass it. Note that we don't have DEBUG logs (although tome, the url log in INFO looks like DEBUG, and the first kind of log looks like INFO).

So in this PR, showInfo and showWarning are replaced with verbosity, which I think is a good improvement towards the standard logging practice, but also introduced a breaking change for some users. I am OK to put it in the next minor release as logs are not core functionalities, and the change is not that big, but I think it is better to keep the previous types of logs the same, which means we keep the plain print in line 57 ~ 59, 66 in _MultiPage.py. I actually like your way of putting them in INFO, but since orderDataProduct (_OncDelivery.py and _DataProductFile.py) still use it, it is better to either change them all or not change them. Other reason is that users might get used to the old print style.

kan-fu · 2025-11-13T19:48:02Z

src/onc/onc.py

-        showInfo: bool = False,
-        showWarning: bool = False,
        outPath: str | Path = "output",
+        verbosity: str = "INFO",


We better keep the default value to "WARNING" to match the previous behavior.

kan-fu · 2025-11-13T21:54:26Z

src/onc/onc.py

+        if self.verbosity in ['INFO', 'DEBUG']:
+            self.showInfo = True
+            self.showWarning = True
+        elif self.verbosity in ['WARNING', 'ERROR']:


In this logic users cannot turn off showWarning. But actually this variable can be removed since it is not used anywhere. And also better to add a else branch to raise a valueError for unexpected verbosity.

kan-fu · 2025-11-13T22:21:26Z

src/onc/modules/_Messages.py

+
+
+def setup_logger(logger_name: str = 'onc-client',
+                 level: int | str = 'DEBUG') -> logging.Logger:


We are actually still supporting the 3.9 version, which does not have this | feature for type hints. I will probably bump it in the next minor release, but for now we have to add from __future__ import annotations.

I could switch to from typing import Union and use Union[int, str] instead.

It is better to keep the | syntax. I already used this in the code (and added that import). It is easier to update after I bump the version. Actually I plan to do that and release a new minor version after I merge your PR.

kan-fu · 2025-11-13T22:22:16Z

src/onc/modules/_MultiPage.py

+                    pageCount += 1
+                    rowCount = self._rowCount(response, service)
+
+                    self.__log.debug(f"Submitting request for page {pageCount} ({rowCount} samples)...")


Here and line 57,58,59 for the print in my other comment.

IanTBlack · 2025-11-25T17:58:25Z

Thanks for the feedback. I will implement your suggestions and make sure things work with black and ruff.

IanTBlack · 2025-11-25T19:34:15Z

There are a couple of different doc string formats in the package. I usually just use the default that PyCharm uses. Which would you prefer moving forward?

Example #1

Example #2

PyCharm Example

kan-fu · 2025-11-25T20:01:19Z

There are a couple of different doc string formats in the package. I usually just use the default that PyCharm uses. Which would you prefer moving forward?

Example #1

Example #2

PyCharm Example

I normally use numpy style (https://oceannetworkscanada.github.io/api-python-client/contributing.html#write-documentation), which is required for the main onc.py file (that is your example 1). Other doc strings are only internally interested, so they don't matter, but it is better to still use the numpy style.

Add functionality for more descriptive console messages and returning…

82e8c89

… requests objects.

IanTBlack changed the title ~~Add functionality for more descriptive console messages and returning~~ Add functionality for more descriptive console messages and return of response objects. Nov 10, 2025

kan-fu requested changes Nov 13, 2025

View reviewed changes



		def setup_logger(logger_name: str = 'onc-client',
		level: int \| str = 'DEBUG') -> logging.Logger:

Add functionality for more descriptive console messages and return of response objects. #69

Are you sure you want to change the base?

Add functionality for more descriptive console messages and return of response objects. #69

Uh oh!

Conversation

IanTBlack commented Nov 10, 2025

Uh oh!

kan-fu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kan-fu Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IanTBlack Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kan-fu Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IanTBlack commented Nov 25, 2025

Uh oh!

IanTBlack commented Nov 25, 2025

Uh oh!

kan-fu commented Nov 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kan-fu left a comment •

edited

Loading

kan-fu Nov 13, 2025 •

edited

Loading

IanTBlack Nov 25, 2025 •

edited

Loading

kan-fu Nov 25, 2025 •

edited

Loading