Skip to content

AP-513 improving order of operations#21

Draft
jason-raitz wants to merge 1 commit intomainfrom
AP-513_log_tracing_part_deux
Draft

AP-513 improving order of operations#21
jason-raitz wants to merge 1 commit intomainfrom
AP-513_log_tracing_part_deux

Conversation

@jason-raitz
Copy link
Contributor

  • co-authored with @danschmidt5189
  • some logs were still not getting request_id logged
  • we believe there an issue with when the two around_performs were called
  • this commit should eliminate that problem

- co-authored with @danschmidt5189
- some logs were still not getting request_id logged
- we believe there an issue with when the two around_performs were
  called
- this commit should eliminate that problem
Copy link
Member

@danschmidt5189 danschmidt5189 left a comment

Choose a reason for hiding this comment

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

Curious about the test failures.

around_perform :log_job_metadata
def request_id
if !defined? @request_id
if arguments.last.is_a?(Hash) && arguments.last.key?(:request_id)
Copy link
Member

Choose a reason for hiding this comment

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

Since it came up on our call, I'll just note you could use find to make this more robust to argument order:

@request_id = args.select { |arg| arg.is_a? Hash }
  .find(-> {{}}) { |arg| arg.key?(:request_id) }
  .fetch(:request_id)

I'm sure there's a more syntactically sugary way to write that, too, but you get the gist. (And yes, it's inefficient to select -> find, I've just done that for clarity.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants