Skip to content

Bug fixes and recursive searching for all matches#2

Merged
s-heppner merged 23 commits intos-heppner:masterfrom
dxvidnrt:master
Jun 28, 2024
Merged

Bug fixes and recursive searching for all matches#2
s-heppner merged 23 commits intos-heppner:masterfrom
dxvidnrt:master

Conversation

@dxvidnrt
Copy link
Copy Markdown
Contributor

@dxvidnrt dxvidnrt commented May 27, 2024

This implements:

  • a feature to search all fitting matches
    recursively
  • an endpoint get_all_matches of the server
  • a bug fix, changing the listen address from
    127.0.0.1 to 0.0.0.0 to work inside of
    docker networks
  • a bug fix, to not empty the matches list and
    actually return all relevant matches

Copy link
Copy Markdown
Owner

@s-heppner s-heppner left a comment

Choose a reason for hiding this comment

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

I think the biggest issue I see at the moment is the cross-dependency you introduced to the semantic_id_resolver.
If possible at all, I would avoid calling that in as a dependency.

If that doesn't work out, my suggestion is to create a completely new repository for the test framework, where you pull in both projects as dependency.

Comment thread config.ini.default
Comment thread requirements.txt Outdated
Comment thread semantic_matcher/service.py Outdated
Comment thread semantic_matcher/service.py Outdated
Comment thread semantic_matcher/service.py Outdated
Remove not used debug function read_root()
Copy link
Copy Markdown
Owner

@s-heppner s-heppner left a comment

Choose a reason for hiding this comment

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

Except for this minor thing, it looks good to me.

Comment thread requirements.txt Outdated
@s-heppner s-heppner changed the title Set up project for bachelors thesis Bug fixes and recursive searching for all matches Jun 27, 2024
@s-heppner
Copy link
Copy Markdown
Owner

I took the liberty to improve your PR title and message, so that we can use it as commit message, when we squash and merge.

Copy link
Copy Markdown
Owner

@s-heppner s-heppner left a comment

Choose a reason for hiding this comment

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

LGTM now

@s-heppner s-heppner merged commit 0df65cc into s-heppner:master Jun 28, 2024
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