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

Refactor registering endpoints #582

Closed
wants to merge 1 commit into from

Conversation

dsblank
Copy link
Member

@dsblank dsblank commented Dec 8, 2024

This PR is the first step in allowing the app to support selectable endpoints, for example, leaving out the openAI LLM requirements. Could also allow an endpoint plugin API.

The next step is to figure out how to configure for the server and frontend. Maybe a config, or environment variable?

@DavidMStraub
Copy link
Member

Let's please discuss such major architecture changes in an issue first. I'm not saying I'm opposed to this, but I don't want to merge something that's a first step before roughly thinking through the subsequent ones.

@DavidMStraub
Copy link
Member

DavidMStraub commented Dec 8, 2024

(It would also be good if we could postpone major refactoring in existing modules until #578 is done, otherwise it will lead to tedious merge conflicts.)

@dsblank
Copy link
Member Author

dsblank commented Dec 8, 2024

I didn't mean for this to be considered for merging! Just some code for discussion.

Regarding adding type hints: I see that you haven't done api/__init__.py yet. I volunteer to do that (possibly on a refactored version). But I'll start a discussion first.

@dsblank dsblank marked this pull request as draft December 8, 2024 19:41
@dsblank
Copy link
Member Author

dsblank commented Dec 8, 2024

Added a place for discussion here: #585

@DavidMStraub
Copy link
Member

I didn't mean for this to be considered for merging! Just some code for discussion.

Got it!

Regarding adding type hints: I see that you haven't done api/init.py yet. I volunteer to do that (possibly on a refactored version). But I'll start a discussion first.

Any help on this PR is more than welcome! I will stop working on it for today in 5 minutes 😆 If something is unclear or discussion needed, you can just add a comment to the PR thread. Thank you!

@dsblank dsblank closed this Dec 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