[pytx] CLI separation: move config and state classes out of cli module #1294
Conversation
github-actions
bot
added
the
python-threatexchange
dougneal
force-pushed
the
pytx-config-state-refactor
branch
from
3fda1f8
to
751d8a1
Compare
dougneal
force-pushed
the
pytx-config-state-refactor
branch
from
751d8a1
to
83c3d38
Compare
There was a problem hiding this comment.
I think it might be better at this exact time to just import the CLI libraries as they are named - this is something that is easy for us to do after the hackathon, but may be disruptive during the hackathon.
That, plus the other considerations on whether we want to promote these interfaces fully make this not clear cut to me.
Is there anything being blocked by these changes that requires them?
| f"Failed to read state for {collab_name}. " | ||
| "You might have to delete it with `threatexchange fetch --clear`" | ||
| ) | ||
| ) from exc |
There was a problem hiding this comment.
I think this is captured by default in modern python, and might explicitly hint you don't want the full stack printed (or maybe that's from None
There was a problem hiding this comment.
Oh ok, I was following a linter recommendation but it may have been an out of date rule
| @@ -32,9 +31,15 @@ | |||
| from threatexchange.signal_type import index | |||
|
|
|||
|
|
|||
| class CliIndexStore: | |||
| class TXStateException(Exception): | |||
There was a problem hiding this comment.
This is going to collide with Nikolay's changes if he has any in flight,
No prob. I'll close this. |
Summary
The library's core functionality and the CLI are separate concerns, so code which doesn't concern the CLI, shouldn't have the appearance of doing so. Move and rename the config and state management classes to reflect this. This will be fundamental to making the API parts make sense.
Test Plan
See what GH CI says...