-
Notifications
You must be signed in to change notification settings - Fork 164
feat(BA-3032): Add Hive router setup in install_halfstack() #6740
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
Conversation
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.
Pull Request Overview
This PR adds Hive Router setup to the install_halfstack() function by generating required configuration files (gateway.config.ts and supergraph.graphql) using the Rover CLI and copying them to the installation directory.
- Uses Rover CLI to compose supergraph schema from
supergraph.yaml - Copies gateway configuration and supergraph schema to installation directory
- Generates files needed for Hive Router container startup
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "--config", | ||
| "configs/graphql/supergraph.yaml", | ||
| ] | ||
| output_path = "docs/manager/graphql-reference/supergraph.graphql" |
Copilot
AI
Nov 12, 2025
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.
The output path is a relative string path that depends on the current working directory. This could fail if the script is not run from the expected project root. Consider using project_root / output_path to make it absolute and consistent with the later file operations.
| output_path = "docs/manager/graphql-reference/supergraph.graphql" | |
| project_root = Path(__file__).resolve().parents[4] | |
| output_path = project_root / "docs/manager/graphql-reference/supergraph.graphql" |
| with open(output_path, "wb") as f: | ||
| f.write(stdout) |
Copilot
AI
Nov 12, 2025
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.
Using synchronous file I/O in an async function. Since aiofiles is already imported in this file (line 23), consider using async with aiofiles.open(output_path, 'wb') as f: await f.write(stdout) for consistency with the async context and to avoid blocking the event loop.
| with open(output_path, "wb") as f: | |
| f.write(stdout) | |
| async with aiofiles.open(output_path, "wb") as f: | |
| await f.write(stdout) |
| self.log_header(f"Wrote supergraph schema to {output_path}") | ||
|
|
||
| base_path = self.install_info.base_path | ||
| project_root = Path(__file__).resolve().parents[4] |
Copilot
AI
Nov 12, 2025
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.
Using magic number [4] to traverse parent directories is fragile and depends on the exact file structure. If the file is moved or the directory structure changes, this will break. Consider using a more robust approach like searching for a marker file (e.g., 'BUILD_ROOT') or defining the project root explicitly in the context.
| ) | ||
| stdout, stderr = await proc.communicate() | ||
| if proc.returncode != 0: | ||
| raise RuntimeError(f"Failed to compose supergraph schema:\n{stderr.decode()}") |
Copilot
AI
Nov 12, 2025
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.
The error message could be more informative by including the command that failed. Consider changing to: raise RuntimeError(f\"Failed to compose supergraph schema using rover CLI:\n{stderr.decode()}\") to make troubleshooting easier.
| raise RuntimeError(f"Failed to compose supergraph schema:\n{stderr.decode()}") | |
| raise RuntimeError( | |
| f"Failed to compose supergraph schema using rover CLI.\n" | |
| f"Command: {' '.join(compose_cmd)}\n" | |
| f"Error output:\n{stderr.decode()}" | |
| ) |
src/ai/backend/install/context.py
Outdated
| async def install_halfstack(self) -> None: | ||
| self.log_header("Installing halfstack...") | ||
|
|
||
| # TODO: 설치할 때 routing url이 달라질수 있으니 인스톨러에서 flag로 세팅하여 routing_url 수정하도록 변경필요 |
Copilot
AI
Nov 12, 2025
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.
TODO comment in Korean may not be accessible to all contributors. Consider translating to English: # TODO: routing URLs may vary during installation; add installer flag to configure routing_url
| # TODO: 설치할 때 routing url이 달라질수 있으니 인스톨러에서 flag로 세팅하여 routing_url 수정하도록 변경필요 | |
| # TODO: Routing URLs may vary during installation; update the installer to allow setting routing_url via a flag. |
1a10632 to
2d8c9ea
Compare
resolves #6739 (BA-3032)
This PR updates the setup process for the Hive Router container.
It uses the Rover CLI to make gateway.config.ts and supergraph.graphql, ensuring the container starts correctly with the required configuration files
Checklist: (if applicable)