Skip to content

Move question answer logic into Session.answer_question()#975

Merged
dhalperi merged 1 commit intomasterfrom
spr/master/fa22b8a7
Apr 6, 2026
Merged

Move question answer logic into Session.answer_question()#975
dhalperi merged 1 commit intomasterfrom
spr/master/fa22b8a7

Conversation

@dhalperi
Copy link
Copy Markdown
Member

@dhalperi dhalperi commented Apr 6, 2026

QuestionBase.answer() previously called _bf_answer_obj(), a free function in internal.py that directly invoked restv2helper and workhelper, bypassing the session. This made it impossible for Session subclasses to override question answering behavior.

Move that logic into Session.answer_question() and have QuestionBase.answer() call it via self._session. Remove the now-unused _bf_answer_obj() function.


Prompt:

Make session.q(params).answer() call back into session so that
subclasses can override the question answer transport.

commit-id:fa22b8a7

@dhalperi dhalperi enabled auto-merge (squash) April 6, 2026 22:22
@batfish-bot
Copy link
Copy Markdown

This change is Reviewable

Copy link
Copy Markdown
Member Author

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

@dhalperi reviewed 3 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on dhalperi).

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 6, 2026

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.79%. Comparing base (4bce0e1) to head (a0c1398).

Files with missing lines Patch % Lines
pybatfish/client/session.py 77.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #975      +/-   ##
==========================================
- Coverage   87.80%   87.79%   -0.02%     
==========================================
  Files          26       26              
  Lines        3773     3769       -4     
  Branches      503      503              
==========================================
- Hits         3313     3309       -4     
  Misses        330      330              
  Partials      130      130              
Files with missing lines Coverage Δ
pybatfish/client/internal.py 100.00% <100.00%> (+11.11%) ⬆️
pybatfish/question/question.py 81.37% <100.00%> (ø)
pybatfish/client/session.py 81.28% <77.77%> (-0.10%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

QuestionBase.answer() previously called _bf_answer_obj(), a free
function in internal.py that directly invoked restv2helper and
workhelper, bypassing the session. This made it impossible for
Session subclasses to override question answering behavior.

Move that logic into Session.answer_question() and have
QuestionBase.answer() call it via self._session. Remove the now-unused
_bf_answer_obj() function.

----

Prompt:
```
Make session.q(params).answer() call back into session so that
subclasses can override the question answer transport.
```

commit-id:fa22b8a7
@dhalperi dhalperi force-pushed the spr/master/fa22b8a7 branch from 2e00e84 to a0c1398 Compare April 6, 2026 22:30
Copy link
Copy Markdown
Member Author

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

@dhalperi reviewed 1 file and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on dhalperi).

@dhalperi dhalperi merged commit a97d160 into master Apr 6, 2026
35 of 36 checks passed
@dhalperi dhalperi deleted the spr/master/fa22b8a7 branch April 6, 2026 22:35
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