-
Notifications
You must be signed in to change notification settings - Fork 8
Addressing small bugs with fixes #15
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ | |
| from autogen_agentchat.base import TaskResult | ||
|
|
||
| # Import predefined models and manager | ||
| from backend.core.agents import AgentManager, IssueRequest, IssueResponse | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't this give an error on the Openshift pod, since it would run from the root of the repository? For a local run, would running the server from root of the repository instead of the backend directory solve this issue?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah if we update the instructions in the README to run from root instead of backend dir then we don't need to do this
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks good overall, If we revert this change and update the README, I'll go ahead and merge this in. |
||
| from core.agents import AgentManager, IssueRequest, IssueResponse | ||
|
|
||
| # Setup logging | ||
| logger = logging.getLogger(__name__) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a clear bug, I don't know how it is working now :)
The point of adding this config file was to make the backend API URL configurable. The react app generates pages and settings during build time and so it was not straightforward to have a runtime environment variable for the backend API. I'm open to suggestions / alternatives to this approach, my understanding here is limited.
We'd have to test any changes by rebuilding the image and testing the deployment on OpenShift. We need Github Actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so the bug I got was basically
config.js file not foundand since the actual file name isconfig.tsonce I updated it, it worked...not sure how it worked on OpenShift if the filename itself doesn't exist?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense!