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

fix: ensure we update job attachments action no matter what #12

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

epmog
Copy link
Contributor

@epmog epmog commented Aug 30, 2023

What was the problem/requirement? (What/Why)

Worker in closed-beta was crashing before marking a job attachments sync action as failed:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/concurrent/futures/_base.py", line 330, in _invoke_callbacks
    callback(self)
  File "/usr/local/lib/python3.9/site-packages/bealine_worker_agent/sessions/actions/sync_input_job_attachments.py", line 140, in _on_done
    session.logger.exception(e)
  File "/usr/local/lib/python3.9/logging/__init__.py", line 1829, in exception
    self.log(ERROR, msg, *args, exc_info=exc_info, **kwargs)
  File "/usr/local/lib/python3.9/logging/__init__.py", line 1844, in log
    self.logger.log(level, msg, *args, **kwargs)
  File "/usr/local/lib/python3.9/logging/__init__.py", line 1512, in log
    self._log(level, msg, args, **kwargs)
  File "/usr/local/lib/python3.9/logging/__init__.py", line 1589, in _log
    self.handle(record)
  File "/usr/local/lib/python3.9/logging/__init__.py", line 1598, in handle
    if (not self.disabled) and self.filter(record):
  File "/usr/local/lib/python3.9/logging/__init__.py", line 806, in filter
    result = f.filter(record)
  File "/usr/local/lib/python3.9/site-packages/openjobio/processing/_action_filter.py", line 130, in filter
    match = filter_matcher.match(record.msg)
TypeError: expected string or bytes-like object

This is because our ojio filter expects the record.msg to be a str, but in reality that filter should be co-ercing the value into a string before using it in a regex.

What was the solution? (How)

Newer worker agents have a newer OJIO version that fixes this: https://github.com/casillas2/openjobio/commit/4cba955dfdac38278ff5895fa5c0591f6dbe7550#diff-bfd9934f32ccd128ff1bbb4c62771fbd25e2b739950bca6a4e80bf5163c568f1R130-R133

We also moved to a try/except/finally clause to ensure that the update action happens

What is the impact of this change?

worker agent doesn't stall out indefinitely since it'll actually mark the action as FAILED.

How was this change tested?

hatch run fmt
hatch run lint
hatch build
hatch run test

Looks like there's no test for this specifically though... if I get some time, I'll add one.

Was this change documented?

N/A

Is this a breaking change?

No

@epmog epmog requested a review from a team as a code owner August 30, 2023 17:00
Copy link
Contributor

@ttblanchard ttblanchard left a comment

Choose a reason for hiding this comment

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

LGTM, Looks like you need to run the formatter to fix the CI.

@gahyusuh gahyusuh force-pushed the fix_exception_logging branch 3 times, most recently from 3478e1b to e7e0055 Compare August 30, 2023 21:25
@epmog epmog changed the title fix: ensure no exceptions are passed to the session logger.exception fix: ensure we update job attachments action no matter what Aug 30, 2023
@gahyusuh gahyusuh merged commit 0d64cbd into mainline Aug 30, 2023
4 checks passed
@gahyusuh gahyusuh deleted the fix_exception_logging branch August 30, 2023 21:43
@jusiskin jusiskin added the bug Something isn't working label Aug 30, 2023
jusiskin pushed a commit to jusiskin/deadline-cloud-worker-agent that referenced this pull request Sep 4, 2024
…-deadline#12)

Updates the requirements on [ruff](https://github.com/astral-sh/ruff) to permit the latest version.
- [Release notes](https://github.com/astral-sh/ruff/releases)
- [Changelog](https://github.com/astral-sh/ruff/blob/main/BREAKING_CHANGES.md)
- [Commits](astral-sh/ruff@v0.0.284...v0.0.288)

---
updated-dependencies:
- dependency-name: ruff
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants