Skip to content

Conversation

@bazarkua
Copy link
Collaborator

Added validation to KPSelector now only query healthy KPs, we now skip any KP whose /meta_knowledge_graph doesn’t return HTTP 200.

Added SPOKE to a hardcoded blacklist (because (CI and PROD endpoints were returning inconsistent results). in Expand/smartapi.py BLACKLISTED_KPS = {'infores:spoke'} and propagated kps_excluded_by_black_list through the cache. KPSelector logs “Blacklisted by ARAX (KP is unstable)”.

Linting/type hygiene: Minor edits to satisfy mypy/ruff in touched modules; no behavior changes beyond the validation/blacklist logic.

Copy link
Collaborator

@edeutsch edeutsch left a comment

Choose a reason for hiding this comment

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

This seems good and fine. Note that I implemented something crude and temporary in ARAX_Expander.py, see 1f4d225

It is unclear whether is it better to do the blocking in expander as I did vs. kp_selector as you did. either is fine I guess.

One minor quibble is that there seemed to be directives from Translator overall to avoid use of the word "blacklist" in favor of "blocklist". Google "blacklist vs blocklist" to find out why.

@bazarkua
Copy link
Collaborator Author

@edeutsch Thanks for suggestion, you're right after Googling the definitions of the blacklist and blocklist, it's more appropriate to use blocklist.
From my understanding last commit 1f4d225 supposed to be temporary? I can add 'infores:automat-robokop' to the block list in my next commit in the same way I did for 'infores:spoke' in the smartapi.py

@edeutsch
Copy link
Collaborator

@edeutsch Thanks for suggestion, you're right after Googling the definitions of the blacklist and blocklist, it's more appropriate to use blocklist.

thanks

From my understanding last commit 1f4d225 supposed to be temporary?

correct.

I can add 'infores:automat-robokop' to the block list in my next commit in the same way I did for 'infores:spoke' in the smartapi.py

not yet, but perhaps later this week yes. Pending discussion on Wednesday AHM.

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.

3 participants