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

Shadowfax #279

Merged
merged 14 commits into from
Sep 9, 2024
Merged

Shadowfax #279

merged 14 commits into from
Sep 9, 2024

Conversation

uhbrar
Copy link
Contributor

@uhbrar uhbrar commented Sep 6, 2024

This adds pathfinder queries into ARAGORN's capabilities. There are still a few things we still want to handle, but this will work for now.

Before merging, we also need to look at how to get the redis working properly.

src/shadowfax.py Outdated
else:
unpinned_node_category = node.get("categories", ["biolink:NamedThing"])[0]
if len(pinned_node_ids) != 2:
logger.info(f"{guid}: Pathfinder queries requrie two pinned nodes.")
Copy link
Contributor

Choose a reason for hiding this comment

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

require

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

logger.info(f"{guid}: Pathfinder queries requrie two pinned nodes.")
return message, 500

async with httpx.AsyncClient(timeout=900) as client:
Copy link
Contributor

Choose a reason for hiding this comment

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

Repeated below, should it be a function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's probably a good idea, yeah.

src/shadowfax.py Outdated
"predicate": "biolink:related_to",
"sources": [
{
"resource_id": "infores:shadowfax",
Copy link
Contributor

Choose a reason for hiding this comment

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

Has this been added to infores?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a debugging thing that slipped through the cracks, just to make it easier to differentiate which edges were actually coming from shadowfax. This has been changed to infores:aragorn

Copy link
Contributor

@cbizon cbizon left a comment

Choose a reason for hiding this comment

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

Minor comments - KL/AT probably the only blocker.

main_edge = uuid.uuid1().hex
before_edge = uuid.uuid1().hex
after_edge = uuid.uuid1().hex
knowledge_graph["edges"][main_edge] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also needs knowledge level / agent type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@uhbrar uhbrar requested a review from cbizon September 6, 2024 22:31
@uhbrar uhbrar merged commit 0f6bc64 into main Sep 9, 2024
1 check passed
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