Skip to content

Addressing small bugs with fixes#15

Open
hemajv wants to merge 1 commit into
redhat-et:mainfrom
hemajv:main
Open

Addressing small bugs with fixes#15
hemajv wants to merge 1 commit into
redhat-et:mainfrom
hemajv:main

Conversation

@hemajv

@hemajv hemajv commented Jul 23, 2025

Copy link
Copy Markdown

When running the setup locally I ran into few bugs for which I have fixes in this PR

@hemajv hemajv requested a review from Shreyanand July 23, 2025 19:07

@Shreyanand Shreyanand left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR! added some discussion points.

Comment thread backend/api/main.py
from autogen_agentchat.base import TaskResult

# Import predefined models and manager
from backend.core.agents import AgentManager, IssueRequest, IssueResponse

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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.
Thanks for the fixes!

Comment thread UI/entrypoint.sh

mkdir -p /opt/app-root/src/config

cat <<EOF > /opt/app-root/src/config/config.js

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Author

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 found and since the actual file name is config.ts once I updated it, it worked...not sure how it worked on OpenShift if the filename itself doesn't exist?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

makes sense!

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