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

Add test coverage for logging outside flow run #8394

Merged
merged 4 commits into from Apr 4, 2023

Conversation

serinamarie
Copy link
Contributor

@serinamarie serinamarie commented Feb 3, 2023

Overview

Adds 4 new tests around the PREFECT_LOGGING_ORION_WHEN_MISSING_FLOW setting to ensure that:

  1. a default setting of warn does not raise
  2. a setting of warn does not raise
  3. a setting of ignore does not raise or warn
  4. a setting of error does not warn

Example

Intended to test cases like:

❯ PREFECT_LOGGING_EXTRA_LOGGERS="my-logger" python example.py    
Traceback (most recent call last):
  File "/Users/mz/dev/prefect/src/prefect/logging/handlers.py", line 279, in emit
    self.get_worker(profile).enqueue(self.prepare(record, profile.settings))
  File "/Users/mz/dev/prefect/src/prefect/logging/handlers.py", line 323, in prepare
    raise MissingContextError(
prefect.exceptions.MissingContextError: Logger 'my-logger' attempted to send logs to Orion without a flow run id. The Orion log handler can only send logs within flow run contexts unless the flow run id is manually provided.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "example.py", line 6, in <module>
    my_logger.info("outside the flow")
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-38/lib/python3.8/logging/__init__.py", line 1434, in info
    self._log(INFO, msg, args, **kwargs)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-38/lib/python3.8/logging/__init__.py", line 1577, in _log
    self.handle(record)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-38/lib/python3.8/logging/__init__.py", line 1587, in handle
    self.callHandlers(record)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-38/lib/python3.8/logging/__init__.py", line 1649, in callHandlers
    hdlr.handle(record)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-38/lib/python3.8/logging/__init__.py", line 950, in handle
    self.emit(record)
  File "/Users/mz/dev/prefect/src/prefect/logging/handlers.py", line 281, in emit
    self.handleError(record)
  File "/Users/mz/dev/prefect/src/prefect/logging/handlers.py", line 293, in handleError
    warnings.warn(exc, stacklevel=8)
TypeError: expected string or bytes-like object

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • This pull request includes tests or only affects documentation.
  • This pull request includes a label categorizing the change e.g. fix, feature, enhancement

@netlify
Copy link

netlify bot commented Feb 3, 2023

Deploy Preview for prefect-docs ready!

Name Link
🔨 Latest commit 9dd3d2f
🔍 Latest deploy log https://app.netlify.com/sites/prefect-docs/deploys/642c5eae2a36060008d4b2ff
😎 Deploy Preview https://deploy-preview-8394--prefect-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@serinamarie serinamarie added the maintenance Chores and other work not directly related to the product label Feb 3, 2023
tests/test_logging.py Outdated Show resolved Hide resolved
tests/test_logging.py Outdated Show resolved Hide resolved
tests/test_logging.py Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2023

This pull request is stale because it has been open 60 days with no activity. To keep this pull request open remove stale label or comment.

@github-actions github-actions bot added the status:stale This may not be relevant anymore label Apr 4, 2023
@zanieb
Copy link
Contributor

zanieb commented Apr 4, 2023

@serinamarie with my suggested changes I do get a failure as expected by reverting the str(exc)

    def handleError(self, record: logging.LogRecord) -> None:
        _, exc, _ = sys.exc_info()
    
        if isinstance(exc, MissingContextError):
            log_handling_when_missing_flow = (
                PREFECT_LOGGING_TO_API_WHEN_MISSING_FLOW.value()
            )
            if log_handling_when_missing_flow == "warn":
                # Warn when a logger is used outside of a run context, the stack level here
                # gets us to the user logging call
>               warnings.warn(exc, stacklevel=8)
E               TypeError: expected string or bytes-like object

src/prefect/logging/handlers.py:302: TypeError

FAILED tests/test_logging.py::TestAPILogHandler::test_does_not_raise_when_logger_outside_of_run_context_with_warn_setting - TypeError: expected string or bytes-like object

@zanieb zanieb marked this pull request as ready for review April 4, 2023 17:18
@zanieb zanieb requested a review from a team as a code owner April 4, 2023 17:18
@zanieb zanieb force-pushed the fix-test-logging-outside-flow-run branch from 2077fe7 to ea2d871 Compare April 4, 2023 17:18
@zanieb
Copy link
Contributor

zanieb commented Apr 4, 2023

I've rebased this onto main let's get it merged.

@github-actions github-actions bot removed the status:stale This may not be relevant anymore label Apr 4, 2023
@zanieb
Copy link
Contributor

zanieb commented Apr 4, 2023

See pytest-dev/pytest#10865

@serinamarie serinamarie merged commit 57ddfbb into main Apr 4, 2023
40 checks passed
@serinamarie serinamarie deleted the fix-test-logging-outside-flow-run branch April 4, 2023 21:24
asmundo pushed a commit to asmundo/prefect that referenced this pull request May 11, 2023
Co-authored-by: Michael Adkins <michael@prefect.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Chores and other work not directly related to the product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants