Added CLI for potpie #233
Added CLI for potpie #233
Conversation
added pytest_cach and poitre.pid in the gitignore added tabulate and pytest in the requirements.txt
WalkthroughThis pull request introduces updates to ignore patterns, dependency management, configuration, and documentation files. New configuration files and metadata have been added for project setup, alongside comprehensive enhancements to the API wrapper, utility functions, and server management components. A Click-based CLI tool is provided with commands for starting/stopping the server, parsing repositories, managing projects, and handling conversations. Additionally, a suite of new unit tests ensures robust error handling and functionality across these components. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as CLI
participant SM as ServerManager
participant D as Docker/Postgres
participant UT as Utility
U->>C: Execute "start" command
C->>SM: start_server()
SM->>UT: check_environment()
UT-->>SM: Environment OK
SM->>D: is_docker_installed() & start_docker()
D-->>SM: Docker ready
SM->>D: check_postgres()
D-->>SM: Postgres available
SM->>SM: run_migrations(), run_server(), run_celery()
SM-->>C: Server running
C-->>U: Display success message
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@dhirenmathur I think there is some error, is it internal or fine let me know. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
app/potpie.py (3)
27-37: Consider environment flexibility or improved fallback strategy.Currently, the code only allows execution when
ENVis specifically set to"development". In case there's a need to run the CLI in other environments (e.g., staging, testing), consider adding configuration options or a fallback strategy to prevent abrupt termination.
350-416: Add limit/offset support for listing projects.You have a TODO indicating a limit is needed for displaying projects. Large numbers of projects could overwhelm the CLI output. Implementing pagination or a maximum display limit would enhance usability.
133-223: Separate server logic from CLI interface.Placing all server management logic in the CLI file can lead to monolithic code, making testing and maintenance more difficult. Consider extracting
start_server,stop_server, and Docker/migration logic into separate modules or services, and letcli()strictly handle user interaction.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)app/potpie.py(1 hunks)requirements.txt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- requirements.txt
🧰 Additional context used
🪛 Ruff (0.8.2)
app/potpie.py
244-244: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
431-431: Use capitalized environment variable DEFAULTUSERNAME instead of defaultUsername
(SIM112)
🔇 Additional comments (3)
app/potpie.py (2)
78-101: Check for Docker container existence before executing PostgreSQL checks.If the named container (
potpie_postgres) doesn't exist or fails to start, this code will raise an error. Consider verifying the container's existence before attempting to runpg_isready.
308-346: Sanitize or validate user inputs used for repository parsing.Users might pass invalid or malicious paths. Consider adding stricter validation or sanitization of the
repoargument to avoid potential injection or unexpected behavior in subsequent steps..gitignore (1)
25-28: Good addition of pytest cache and PID file ignores.Ignoring
.pytest_cacheandpoitre.pidhelps maintain a clean repository, avoiding clutter from local test artifacts and process ID files.
|
@DeepeshKalura can we simplify : to something like |
@dhirenmathur 1. Bash Script Wrapper (Simplest Approach) Therfore, CLIs are not created like that. 2. setuptools (setup.py) – The Old Way (2005-2010) this still works i think but now people use prefer pyproject.toml instead. 3. Modern Approach – pyproject.toml (Recommended Today) Yeah in this approach we generally don't have requirement.txt file but we can work with both |
|
@dhirenmathur Since you didn’t respond in a timely manner, I will proceed with managing this aspect accordingly. For reference, you can review the official documentation on Python packaging: Key Issue:You intend to use Next Steps:Since you hadn’t previously mentioned the need for a package but require it now, I want to confirm the approach. I am opting for Option 3, which involves:
|
|
Thanks @DeepeshKalura third option sounds good. |
…ile creation functionality, and enhance tests
…on functionalities
…configure pytest options
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
cli/src/potpie/utility.py (2)
2-2: Remove unused import.The
loggingmodule is imported but never used in this file.-import logging🧰 Tools
🪛 Ruff (0.8.2)
2-2:
loggingimported but unusedRemove unused import:
logging(F401)
26-26: Use uppercase for environment variable name.Environment variables should use uppercase letters and underscores.
- return os.getenv("defaultUsername", "defaultuser") + return os.getenv("DEFAULT_USERNAME", "defaultuser")🧰 Tools
🪛 Ruff (0.8.2)
26-26: Use capitalized environment variable
DEFAULTUSERNAMEinstead ofdefaultUsername(SIM112)
cli/test/test_api_wrapper.py (1)
13-20: Simplify mock responses using fixtures.Consider using pytest fixtures to reduce duplication in mock response classes.
@pytest.fixture def mock_response(): def _mock_response(status_code, data): class MockResponse: @property def status_code(self): return status_code def json(self): return data return MockResponse() return _mock_responseAlso applies to: 23-29
cli/src/potpie/api_wrapper.py (1)
106-138: Improve async interaction implementation.The async implementation could be improved:
- Add timeout handling
- Use async context manager pattern
- Add backoff/retry logic for failed requests
async def interact_with_agent(self, conversation_id: str, content: str): url = f"{self.base_url}/api/v1/conversations/{conversation_id}/message/" + timeout = aiohttp.ClientTimeout(total=30) - async with aiohttp.ClientSession() as session: + async with aiohttp.ClientSession(timeout=timeout) as session:🧰 Tools
🪛 Ruff (0.8.2)
126-126: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
cli/.gitignore(1 hunks)cli/.python-version(1 hunks)cli/LICENSE(1 hunks)cli/README.md(1 hunks)cli/contributing.md(1 hunks)cli/pyproject.toml(1 hunks)cli/src/potpie/api_wrapper.py(1 hunks)cli/src/potpie/utility.py(1 hunks)cli/test/test_api_wrapper.py(1 hunks)cli/test/test_utility.py(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- cli/README.md
- cli/.python-version
- cli/.gitignore
- cli/LICENSE
- cli/contributing.md
- cli/pyproject.toml
🧰 Additional context used
🪛 Ruff (0.8.2)
cli/test/test_api_wrapper.py
43-43: pytest.raises(Exception) should be considered evil
(B017)
99-99: pytest.raises(Exception) should be considered evil
(B017)
178-178: pytest.raises(Exception) should be considered evil
(B017)
cli/test/test_utility.py
40-40: Redefinition of unused test_pid_file_creation from line 13
(F811)
cli/src/potpie/utility.py
2-2: logging imported but unused
Remove unused import: logging
(F401)
26-26: Use capitalized environment variable DEFAULTUSERNAME instead of defaultUsername
(SIM112)
cli/src/potpie/api_wrapper.py
126-126: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
cli/src/potpie/utility.py (1)
31-31: Adopt uppercase environment variable naming.Use a capitalized environment variable (e.g.,
DEFAULTUSERNAME) instead ofdefaultUsernamefor consistency and clarity.- return os.getenv("defaultUsername", "defaultuser") + return os.getenv("DEFAULTUSERNAME", "defaultuser")🧰 Tools
🪛 Ruff (0.8.2)
31-31: Use capitalized environment variable
DEFAULTUSERNAMEinstead ofdefaultUsername(SIM112)
cli/test/test_utility.py (1)
75-77: Parameterize environment variable usage for more robust coverage.Currently, the test only checks the default user scenario. Consider adding a test case for when
os.getenv("defaultUsername")is set.cli/src/potpie/server_manager.py (3)
14-36: Consider renaming the customEnvironmentErrorclass.Python 3 no longer has a built-in
EnvironmentError, but historically it was part of Python 2.x and can still cause confusion by overshadowing the old built-in. Renaming it (e.g.,InvalidEnvironmentError) would help avoid ambiguity.
157-158: Consider chaining exceptions withraise ... from e.When re-raising a caught exception as a different one, Python best practice is to use
raise SomeError(...) from eto preserve the original traceback:except subprocess.CalledProcessError as e: raise DockerError("Failed to start Docker containers") from eThis helps distinguish the original error from new ones in exception handling.
Also applies to: 184-185, 210-211, 215-217, 247-248, 309-316, 342-343, 350-351, 358-359
🧰 Tools
🪛 Ruff (0.8.2)
157-157: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
286-288: Possible concurrency conflict if another server is starting.The code checks a single PID file to detect if the server is running. In concurrent environments or multiple server instances, this could be unreliable. You might want to implement a unique or lock-based mechanism to handle multiple starts.
cli/test/test_server_manager.py (1)
276-276: Useis not Noneover!= None.PEP 8 style guidelines recommend using
is not Nonewhen checking againstNone:-if expected_exception!=None: +if expected_exception is not None:🧰 Tools
🪛 Ruff (0.8.2)
276-276: Comparison to
Noneshould becond is not NoneReplace with
cond is not None(E711)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cli/src/potpie/server_manager.py(1 hunks)cli/src/potpie/utility.py(1 hunks)cli/test/test_server_manager.py(1 hunks)cli/test/test_utility.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
cli/src/potpie/utility.py
2-2: logging imported but unused
Remove unused import: logging
(F401)
31-31: Use capitalized environment variable DEFAULTUSERNAME instead of defaultUsername
(SIM112)
cli/test/test_server_manager.py
276-276: Comparison to None should be cond is not None
Replace with cond is not None
(E711)
cli/src/potpie/server_manager.py
157-157: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
184-184: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
211-211: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
215-217: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
241-241: Undefined name e
(F821)
247-247: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
274-274: Undefined name e
(F821)
315-315: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
342-342: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
350-350: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
358-358: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
cli/test/test_utility.py
34-34: Redefinition of unused test_pid_file_creation from line 7
(F811)
🔇 Additional comments (4)
cli/src/potpie/utility.py (2)
12-15: Verify the attribute check forlog_file.Using
if not hasattr(self, "log_file"):is potentially misleading because you never store alog_fileattribute, only_server_log_fileand_celery_log_file. Consider revising the attribute check or removing it if unneeded.
25-27: Make the base URL configurable.This was suggested in a past review. Replacing the hardcoded URL with an environment variable or configuration file would help accommodate different environments (production, staging, development).
cli/src/potpie/server_manager.py (1)
70-80: Environment check logic is fine, but consider adding more specific logging.The code correctly checks for
developmentenvironment, but you might consider adding more detailed logging or exceptions for other environments to help users understand why they can't proceed.cli/test/test_server_manager.py (1)
1-284: Overall test coverage looks good!These tests are fairly comprehensive, covering Docker checks, PostgreSQL checks, migrations, server start, and stop flows. Great job ensuring critical paths and errors are tested.
🧰 Tools
🪛 Ruff (0.8.2)
276-276: Comparison to
Noneshould becond is not NoneReplace with
cond is not None(E711)
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (11)
cli/src/potpie/server_manager.py (8)
60-66: Consider annotatingServerManagerconstructor parameters.
It might be good to clarify the expected types for the constructor’s attributes, such aspid_file,server_log, andcelery_log, explicitly so that others can easily maintain and extend this class.
274-276: Misleading error message referencing “Celery” instead of “Server.”
This block appears to handle the Gunicorn server process but raises a “Start Celery” error message, which can confuse readers.if(server_process.returncode != 0): - raise StartServerError(f" Start Celery Failed: Return Code: {server_process.returncode} with error: {server_process.stderr}") + raise StartServerError( + f"Failed to start Gunicorn server (RC: {server_process.returncode}): {server_process.stderr}" + )
283-284: Enhance traceback preservation by raising from the original exception.
Raising from the original exception preserves its context for better debugging.except Exception as e: - raise StartServerError(f" Start Server Failed: {e}") + raise StartServerError(f"Start Server Failed: {e}") from e🧰 Tools
🪛 Ruff (0.8.2)
284-284: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
312-318: Useraise ... from eto preserve original exception context.
When re-raising an exception in this block, append “from e” to distinguish it from new exceptions in the exception handler.except Exception as e: logging.error(f"Startup failed: {e}") ... - raise StartServerError(f"Startup failed: {e}") + raise StartServerError(f"Startup failed: {e}") from e🧰 Tools
🪛 Ruff (0.8.2)
318-318: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
343-345: Preserve the original exception when raisingStopServerError.
Again, re-raising with “from e” helps maintain the original stack trace and eases debugging.except Exception as e: logging.warning(f"Server process already terminated {e}") - raise StopServerError(f"Server process already terminated {e}") + raise StopServerError(f"Server process already terminated {e}") from e🧰 Tools
🪛 Ruff (0.8.2)
345-345: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
353-353: Raise from the caughtCalledProcessErrorfor better context.
Maintaining the original context is helpful if Docker teardown fails.except subprocess.CalledProcessError as e: logging.error(f"Failed to stop Docker containers: {e}") - raise StopServerError(f"Failed to stop Docker containers: {e}") + raise StopServerError(f"Failed to stop Docker containers: {e}") from e🧰 Tools
🪛 Ruff (0.8.2)
353-353: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
361-361: Improve debugging by raising from the original exception context.except Exception as e: logging.error(f"Error during shutdown: {e}") - raise StopServerError(f"Error during shutdown: {e}") + raise StopServerError(f"Error during shutdown: {e}") from e🧰 Tools
🪛 Ruff (0.8.2)
361-361: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
369-369: Minor typo in the docstring.
“aff the frames” seems like a small typo. Perhaps use “add the frames.”note: we can also aff the frames +note: we can also add the framescli/test/test_utility.py (1)
48-56: Add test cases for edge cases in base URL validation.The current test cases don't cover all edge cases for URL validation.
Add these test cases to cover more scenarios:
[ ("http://localhost:8001", True), ("http://localhost:8000", False), ("http://example.com", False), ("http://localhost:8001/api", False), + ("https://localhost:8001", False), + ("localhost:8001", False), + ("", False), + (None, False) ],cli/src/potpie/api_wrapper.py (2)
105-105: Add return type hint for async generator.The async method should specify its return type.
- async def interact_with_agent(self, conversation_id: str, content: str): + async def interact_with_agent(self, conversation_id: str, content: str) -> AsyncGenerator[str, None]:
125-128: Improve JSON decode error handling.The error handling for JSON decode errors could be more informative.
- except json.JSONDecodeError as e: - # Ignore this because they are just empty man - logging.debug("Received empty or invalid JSON chunk, skipping") + except json.JSONDecodeError: + logging.debug( + "Received empty or invalid JSON chunk, skipping", + extra={"chunk": line[0].decode() if line[0] else "empty"} + )🧰 Tools
🪛 Ruff (0.8.2)
125-125: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cli/src/potpie/api_wrapper.py(1 hunks)cli/src/potpie/server_manager.py(1 hunks)cli/src/potpie/utility.py(1 hunks)cli/test/test_utility.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
cli/src/potpie/api_wrapper.py
3-3: time imported but unused
Remove unused import: time
(F401)
125-125: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
cli/src/potpie/utility.py
30-30: Use capitalized environment variable DEFAULTUSERNAME instead of defaultUsername
(SIM112)
cli/src/potpie/server_manager.py
157-157: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
184-184: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
211-211: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
215-217: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
248-248: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
284-284: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
318-318: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
345-345: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
353-353: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
361-361: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
…n up test_utility.py, and enhance README with CLI usage instructions
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
cli/src/potpie/server_manager.py (1)
241-244:⚠️ Potential issueFix potential race condition in Celery process check.
Checking
returncodeimmediately afterPopenis unreliable as the process hasn't had time to start.Apply this diff to implement proper process monitoring:
- if celery_process.returncode != 0: - raise StartServerError( - f" Start Celery Failed: Return Code: {celery_process.returncode} with error: {celery_process.stderr}" - ) + # Wait briefly for immediate failures + time.sleep(1) + if celery_process.poll() is not None: + if celery_process.returncode != 0: + raise StartServerError( + f"Start Celery failed immediately (Return Code: {celery_process.returncode})" + ) from None
🧹 Nitpick comments (12)
cli/src/potpie/api_wrapper.py (2)
100-100: Add return type hint.The method is missing a return type hint, which is inconsistent with other methods in the class.
Apply this diff to add the return type hint:
- def available_agents(self, system_agent: bool = True): + def available_agents(self, system_agent: bool = True) -> List[dict]:
163-198: Consider implementing backoff and retry for streaming interactions.The
interact_with_agentmethod handles streaming responses but doesn't implement any retry mechanism for temporary network issues or rate limits.Consider implementing exponential backoff and retry logic for transient failures. Here's an example implementation:
from tenacity import retry, stop_after_attempt, wait_exponential @retry( stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=4, max=10), retry=retry_if_exception_type((aiohttp.ClientError, asyncio.TimeoutError)) ) async def interact_with_agent(self, conversation_id: str, content: str): # ... existing implementation ...This would require adding
tenacityto your requirements.cli/src/potpie/main.py (3)
23-32: Allow configuring the loading animation duration instead of a fixed 5s.
While a fixed 5-second animation may be acceptable in some cases, consider making it configurable or cancelable to improve user experience, especially when parsing finishes earlier or needs more time.def loading_animation(message: str, duration: float = 5.0): ... - while (time.time() - start_time) < 5: + while (time.time() - start_time) < duration: ...
179-179: Remove unused local variablebase_url.
The variablebase_urlis never used. Consider removing it to keep the code clean.- base_url: str = Utility.base_url()🧰 Tools
🪛 Ruff (0.8.2)
179-179: Local variable
base_urlis assigned to but never usedRemove assignment to unused variable
base_url(F841)
285-337: Consider robust error handling for the interactive chat loop.
The interactive chat loop will break on any exception or keyboard interrupt. Consider handling additional network/time-out scenarios and gracefully resuming or re-promoting user input.cli/test/test_server_manager.py (1)
306-306: Use identity comparison withNone.
Python encourages usingis not Noneinstead of!= None.- if expected_exception != None: + if expected_exception is not None:🧰 Tools
🪛 Ruff (0.8.2)
306-306: Comparison to
Noneshould becond is not NoneReplace with
cond is not None(E711)
cli/src/potpie/server_manager.py (2)
306-311: Improve startup sequence and logging.The startup sequence has a few issues:
is_runningis set before actually starting services- Logging messages are inconsistent
Apply this diff:
- self.is_running = True - logging.info("🚀 Services started:") + logging.info("🚀 Starting services...") self.run_server() logging.info(f"Server is up and running at {Utility.base_url}") self.run_celery() - logging.info("Startup is running") + logging.info("All services started successfully") + self.is_running = True
362-374: Improve signal handler documentation.The signal handler's docstring has a typo and incomplete information.
Apply this diff:
- """Handle shutdown signal, - - Args: - signum: Signal number - - note: we can also aff the frames - - """ + """Handle shutdown signal gracefully. + + Args: + signum: Signal number received from the system + + Note: + This handler ensures clean shutdown of all services + when receiving system signals like SIGTERM or SIGINT. + """cli/README.md (4)
1-4: Enhance the introduction section.The introduction could be more informative by including installation instructions and prerequisites.
Add installation and prerequisites sections:
# **Poitre CLI Tool** A command-line interface (CLI) tool to manage and interact with the Poitre application, including managing servers, parsing repositories, listing projects, and interacting with conversations. +## **Prerequisites** + +- Python 3.8 or higher +- Docker and Docker Compose +- PostgreSQL client tools + +## **Installation** + +```bash +pip install -e . +```
38-44: Clarify repository path requirements.The documentation should specify the format and requirements for the repository path.
Add more details about the repository path:
poitre parse <repository_path> --branch <branch_name>
- Parses a local repository for the specified branch.
- Arguments:
repository_path: Path to the repository on the local machine.
repository_path: Absolute or relative path to the Git repository on the local machine.
- Must be a valid Git repository
- Repository must be accessible with read permissions
--branch: (Optional) Name of the branch to parse (default:main).--- `63-64`: **Fix command formatting.** Remove extra space in the command. ```diff -poitre conversation list +poitre conversation list
69-70: Fix command formatting.Remove extra space in the command.
-poitre conversation create <title> +poitre conversation create <title>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
cli/README.md(1 hunks)cli/pyproject.toml(1 hunks)cli/src/potpie/api_wrapper.py(1 hunks)cli/src/potpie/main.py(1 hunks)cli/src/potpie/server_manager.py(1 hunks)cli/src/potpie/utility.py(1 hunks)cli/test/test_api_wrapper.py(1 hunks)cli/test/test_server_manager.py(1 hunks)cli/test/test_utility.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cli/test/test_utility.py
- cli/pyproject.toml
🧰 Additional context used
🪛 Ruff (0.8.2)
cli/test/test_api_wrapper.py
43-43: pytest.raises(Exception) should be considered evil
(B017)
99-99: pytest.raises(Exception) should be considered evil
(B017)
178-178: pytest.raises(Exception) should be considered evil
(B017)
241-241: pytest.raises(Exception) should be considered evil
(B017)
245-245: Pointless comparison. Did you mean to assign a value? Otherwise, prepend assert or remove it.
(B015)
cli/src/potpie/main.py
179-179: Local variable base_url is assigned to but never used
Remove assignment to unused variable base_url
(F841)
181-181: Use capitalized environment variable DEFAULTUSERNAME instead of defaultUsername
(SIM112)
cli/src/potpie/utility.py
35-35: Use capitalized environment variable DEFAULTUSERNAME instead of defaultUsername
(SIM112)
cli/src/potpie/server_manager.py
158-158: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
185-185: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
212-212: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
216-218: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
250-250: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
287-287: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
319-319: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
344-344: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
352-352: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
360-360: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
cli/test/test_server_manager.py
306-306: Comparison to None should be cond is not None
Replace with cond is not None
(E711)
🔇 Additional comments (5)
cli/test/test_api_wrapper.py (1)
43-44: Specify expected exception types in tests.Using bare
Exceptioninpytest.raisesis too broad and could mask unexpected errors. Specify the exact exception types you expect to be raised.Apply this diff to fix all occurrences:
- with pytest.raises(Exception): + with pytest.raises((requests.exceptions.RequestException, ValueError)): api_wrapper.parse_project("repo_path")Also applies to: 99-100, 178-179, 241-242
🧰 Tools
🪛 Ruff (0.8.2)
43-43:
pytest.raises(Exception)should be considered evil(B017)
cli/src/potpie/utility.py (2)
35-35: Use uppercase environment variable naming here as well.
This mirrors a previous suggestion for environment variables, but it's still valid for consistent best practices.- return os.getenv("defaultUsername", "defaultuser") + return os.getenv("DEFAULTUSERNAME", "defaultuser")🧰 Tools
🪛 Ruff (0.8.2)
35-35: Use capitalized environment variable
DEFAULTUSERNAMEinstead ofdefaultUsername(SIM112)
67-67: Remove or replace debug print statement.
Printing the PID file path might be better handled with a logging statement to avoid unintended console output.- print(pid_file) + import logging + logging.debug(f"PID file path: {pid_file}")cli/test/test_server_manager.py (1)
1-314: Test coverage looks comprehensive.
These tests thoroughly cover Docker, PostgreSQL, migrations, server start/stop, and various failure scenarios. Great job ensuring robust coverage!🧰 Tools
🪛 Ruff (0.8.2)
306-306: Comparison to
Noneshould becond is not NoneReplace with
cond is not None(E711)
cli/src/potpie/server_manager.py (1)
14-61: LGTM! Well-structured exception hierarchy.The exception classes follow a clean hierarchy with descriptive messages and proper documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
cli/src/potpie/server_manager.py (2)
261-264:⚠️ Potential issueFix race condition in Celery process check.
The returncode check immediately after starting the process is unreliable:
- if celery_process.returncode not in [0, None]: - raise StartServerError( - f" Start Celery Failed: with error: {celery_process.stderr}" - ) + time.sleep(1) # Wait briefly for immediate failures + if celery_process.poll() is not None: + if celery_process.returncode != 0: + raise StartServerError( + f"Start Celery failed immediately (Return Code: {celery_process.returncode})" + ) from None
205-205:⚠️ Potential issueAdd proper exception chaining throughout the file.
Multiple locations are missing proper exception chaining, which can lose valuable debugging information:
Apply this pattern to all similar locations:
- raise PostgresError("Failed to check PostgreSQL status") + raise PostgresError("Failed to check PostgreSQL status") from eThis applies to lines: 205, 232, 236-238, 270, 308, 339, 364, and 372.
Also applies to: 232-232, 236-238, 270-270, 308-308, 339-339, 364-364, 372-372
🧰 Tools
🪛 Ruff (0.8.2)
205-205: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
🧹 Nitpick comments (9)
cli/src/potpie/api_wrapper.py (1)
189-193: Improve error handling in JSON decoding.The exception variable
eis used in the logging statement but could provide more context.Apply this diff:
except json.JSONDecodeError as e: - logging.debug( - "Received empty or invalid JSON chunk, skipping %s", e - ) + logging.debug("Invalid JSON chunk: %s. Error: %s", line[0], e) continuecli/src/potpie/main.py (2)
37-38: Make loading animation timeout configurable.The 5-second timeout is hardcoded and might not be suitable for all operations.
Apply this diff:
+ DEFAULT_TIMEOUT = 5 # seconds + def loading_animation(message: str): """Display loading animation for five seconds""" spinner = itertools.cycle(["⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷"]) start_time = time.time() - while (time.time() - start_time) < 5: + while (time.time() - start_time) < DEFAULT_TIMEOUT:
153-155: Improve project deletion confirmation message.The confirmation message could be more explicit about the irreversible nature of deletion.
Apply this diff:
confirm = click.confirm( - f"Delete {selected_project['repo_name']} with (ID: {selected_project_id})?" + f"Are you sure you want to delete {selected_project['repo_name']} (ID: {selected_project_id})? This action cannot be undone" )cli/test/test_server_manager.py (2)
132-132: Improve boolean comparison.Avoid explicit comparison to
Truein boolean expressions.Apply this diff:
- assert server_manager.check_postgres() == True + assert server_manager.check_postgres()🧰 Tools
🪛 Ruff (0.8.2)
132-132: Avoid equality comparisons to
True; useif server_manager.check_postgres():for truth checksReplace with
server_manager.check_postgres()(E712)
316-316: Fix None comparison.Use
is not Noneinstead of!= Nonefor identity comparison with None.Apply this diff:
- if expected_exception != None: + if expected_exception is not None:🧰 Tools
🪛 Ruff (0.8.2)
316-316: Comparison to
Noneshould becond is not NoneReplace with
cond is not None(E711)
cli/src/potpie/server_manager.py (4)
25-75: Add type hints to exception classes for better maintainability.Consider adding type hints to the message parameters:
- def __init__(self, message="Starting the server has an error"): + def __init__(self, message: str = "Starting the server has an error"):Apply this pattern to all exception classes.
77-86: Enhance class documentation and type hints.Add return type hints and improve the docstring with attributes documentation:
class ServerManager: - """A class to manage the Potpie development server.""" + """A class to manage the Potpie development server. + + Attributes: + pid_file (str): Path to the PID file + server_log (str): Path to the server log file + celery_log (str): Path to the celery log file + """ - def __init__(self): + def __init__(self) -> None:
132-137: Remove unused process variable from Docker startup.The
processvariable from the context manager is not used:- with subprocess.Popen( + subprocess.Popen( ["docker", "compose", "up", "-d"], stdout=subprocess.PIPE, stderr=subprocess.PIPE, - ) as process: + ) logging.info("Docker Process is started...")🧰 Tools
🪛 Ruff (0.8.2)
136-136: Local variable
processis assigned to but never usedRemove assignment to unused variable
process(F841)
297-302: Simplify nested if statements in server process check.The nested if statements can be simplified:
- if server_process.poll() is not None: - if server_process.returncode != 0: - raise StartServerError( - f"Start server (Return Code: {server_process.returncode})" - ) from None + if server_process.poll() is not None and server_process.returncode != 0: + raise StartServerError( + f"Start server failed (Return Code: {server_process.returncode})" + ) from None🧰 Tools
🪛 Ruff (0.8.2)
297-298: Use a single
ifstatement instead of nestedifstatements(SIM102)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cli/pyproject.toml(1 hunks)cli/src/potpie/api_wrapper.py(1 hunks)cli/src/potpie/main.py(1 hunks)cli/src/potpie/server_manager.py(1 hunks)cli/src/potpie/utility.py(1 hunks)cli/test/test_server_manager.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/pyproject.toml
🧰 Additional context used
🪛 Ruff (0.8.2)
cli/src/potpie/server_manager.py
136-136: Local variable process is assigned to but never used
Remove assignment to unused variable process
(F841)
205-205: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
232-232: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
236-238: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
270-270: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
297-298: Use a single if statement instead of nested if statements
(SIM102)
308-308: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
339-339: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
364-364: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
372-372: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
cli/src/potpie/utility.py
44-44: Use capitalized environment variable DEFAULTUSERNAME instead of defaultUsername
(SIM112)
cli/test/test_server_manager.py
132-132: Avoid equality comparisons to True; use if server_manager.check_postgres(): for truth checks
Replace with server_manager.check_postgres()
(E712)
316-316: Comparison to None should be cond is not None
Replace with cond is not None
(E711)
🔇 Additional comments (4)
cli/src/potpie/utility.py (3)
44-44: Use uppercase for environment variable names.The environment variable
defaultUsernamedoesn't follow the conventional uppercase naming pattern for environment variables.Apply this diff to follow the convention:
- return os.getenv("defaultUsername", "defaultuser") + return os.getenv("POTPIE_DEFAULT_USERNAME", "defaultuser")🧰 Tools
🪛 Ruff (0.8.2)
44-44: Use capitalized environment variable
DEFAULTUSERNAMEinstead ofdefaultUsername(SIM112)
57-61: Improve file creation security.The current file creation method might be vulnerable to race conditions and doesn't set proper file permissions.
Apply this diff to improve security:
- with open(log_file, "w", encoding="utf-8") as _: - pass + import stat + flags = os.O_CREAT | os.O_WRONLY + mode = stat.S_IRUSR | stat.S_IWUSR + try: + fd = os.open(log_file, flags, mode) + os.close(fd) + except OSError as e: + logging.error(f"Failed to create log file: {e}") + raise
76-76: Remove debug print statement.The print statement should be replaced with proper logging.
Apply this diff:
- print(pid_file) + logging.debug(f"Created PID file at: {pid_file}")cli/src/potpie/api_wrapper.py (1)
106-106: Add return type hint.The method is missing a return type hint.
Apply this diff:
- def available_agents(self, system_agent: bool = True): + def available_agents(self, system_agent: bool = True) -> dict:
yeah, my mistake it is a shutdown command. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
cli/src/potpie/main.py (5)
1-8: Enhance module docstring with command structure.The docstring could be more helpful by including a brief overview of available commands and their purposes.
Add command structure to the docstring:
""" Potpie CLI Tool This module provides a command-line interface (CLI) for managing and interacting with the Potpie application. Potpie allows users to parse repositories, manage projects, start conversations, and communicate with AI agents. +Available Commands: + start Start the server and related services + stop Stop the server and related services + parse Parse a local repository + projects List and manage projects + conversation Manage conversations with agents """
29-30: Consider dependency injection for better testability.Global instances of
ServerManagerandApiWrappermake unit testing difficult. Consider passing these as parameters to the CLI commands.-server_manager = ServerManager() -api_wrapper = ApiWrapper() + +def get_server_manager(): + return ServerManager() + +def get_api_wrapper(): + return ApiWrapper() + +@click.pass_context +def cli(ctx): + """CLI tool for managing the potpie application""" + ctx.obj = { + 'server_manager': get_server_manager(), + 'api_wrapper': get_api_wrapper() + }
33-42: Improve loading animation flexibility.The loading animation has a fixed 5-second duration and can't be cancelled early. This could lead to unnecessary waiting or premature termination.
-def loading_animation(message: str): +def loading_animation(message: str, duration: float = 5.0, check_completion=None): """Display loading animation for five seconds""" spinner = itertools.cycle(["⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷"]) start_time = time.time() - while (time.time() - start_time) < 5: + while (time.time() - start_time) < duration: sys.stdout.write(f"\r{message} {next(spinner)}") sys.stdout.flush() time.sleep(0.1) + if check_completion and check_completion(): + break sys.stdout.write("\r\033[K")
126-138: Add pagination for project listing.Large number of projects could make the output unwieldy. Consider adding pagination support.
+@click.option('--page', default=1, help='Page number') +@click.option('--per-page', default=10, help='Items per page') -def projects(delete): +def projects(delete, page, per_page): """List all projects and optionally delete selected projects""" try: projects = api_wrapper.get_list_of_projects() if not delete: + start_idx = (page - 1) * per_page + end_idx = start_idx + per_page + page_projects = projects[start_idx:end_idx] # Standard project listing table_data = [ [project["id"], project["repo_name"], project["status"]] - for project in projects + for project in page_projects ] table = tabulate( table_data, headers=["ID", "Name", "Status"], tablefmt="fancy_grid" ) click.echo(table) + total_pages = (len(projects) + per_page - 1) // per_page + click.echo(f"\nPage {page}/{total_pages}")
349-351: Add logging configuration.Configure logging to help with debugging and monitoring.
if __name__ == "__main__": + logging.basicConfig( + level=os.getenv('LOG_LEVEL', 'INFO'), + format='%(asctime)s - %(name)s - %(levelname)s - %(message)s' + ) cli()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cli/src/potpie/main.py(1 hunks)
🔇 Additional comments (2)
cli/src/potpie/main.py (2)
59-59: Fix incorrect error message.The error message mentions "startup" but this is in the stop command.
167-167: Fix incorrect status code access.The status code is accessed directly from the response.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…cks and improve logging
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
cli/src/potpie/main.py (5)
13-13: Configure logging for better diagnostics.The logging module is imported but not configured. This could lead to missing important log messages.
Add this configuration after the imports:
+logging.basicConfig( + level=logging.INFO, + format='%(asctime)s - %(name)s - %(levelname)s - %(message)s' +)
33-42: Parameterize loading animation duration.The loading animation has a hard-coded duration of 5 seconds which may not be suitable for all operations.
Apply this diff to make the duration configurable:
-def loading_animation(message: str): +def loading_animation(message: str, duration: float = 5.0): """Display loading animation for five seconds""" spinner = itertools.cycle(["⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷"]) start_time = time.time() - while (time.time() - start_time) < 5: + while (time.time() - start_time) < duration:
65-68: Improve error handling in stop command.The stop command should handle specific exceptions and provide more informative error messages.
Apply this diff to improve error handling:
try: server_manager.stop_server() + except ConnectionError as e: + logging.error("Failed to connect to server: %s", e) + click.secho("Server might not be running", fg="yellow") except Exception as e: logging.error("Error during shutdown: %s", e) + click.secho("Failed to stop server", fg="red")
174-177: Enhance error messages in projects command.The error messages could be more user-friendly and provide guidance on resolution.
Apply this diff to improve error messages:
except requests.RequestException as e: - logging.error(f"Network error occurred: {e}") + logging.error("Network error occurred: %s", e) + click.secho("Failed to connect to server. Please check your network connection and server status.", fg="red") except Exception as e: - logging.error(f"An unexpected error occurred: {e}") + logging.error("An unexpected error occurred: %s", e) + click.secho("An error occurred. Please try again or contact support if the issue persists.", fg="red")
362-364: Add error handling to main entry point.The main entry point should handle exceptions and provide a clean exit.
Apply this diff to improve error handling:
if __name__ == "__main__": - cli() + try: + cli() + except KeyboardInterrupt: + click.echo("\nOperation cancelled by user") + sys.exit(1) + except Exception as e: + logging.error("Fatal error: %s", e) + click.secho("An unexpected error occurred", fg="red") + sys.exit(1)
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
cli/src/potpie/main.py (4)
49-60: 🛠️ Refactor suggestionAdd server status check and improve error handling.
The start command needs two improvements:
- Check if the server is already running before starting it.
- Use the
handle_api_errordecorator for consistent error handling.Apply this diff:
@cli.command() +@handle_api_error def start(): """Start the server and all related services""" click.secho("Poitre server starting...", fg="blue", bold=True) try: + if server_manager.is_running(): + click.secho("Server is already running.", fg="yellow", bold=True) + return signal.signal(signal.SIGINT, server_manager.handle_shutdown) signal.signal(signal.SIGTERM, server_manager.handle_shutdown) server_manager.start_server() click.secho("Poitre server started successfully.", fg="green", bold=True) except Exception as e: logging.error("Error during startup: %s", e)
71-119: 🛠️ Refactor suggestionImprove parsing progress feedback and error handling.
The parse command needs several improvements:
- Make max attempts configurable.
- Show actual progress during parsing.
- Use the
handle_api_errordecorator.Apply this diff:
@cli.command() @click.argument("repo") @click.option("--branch", default="main", help="Branch name") +@handle_api_error def parse(repo, branch): """Parse a Local repository currently""" + max_attempts = int(os.getenv('PARSE_MAX_ATTEMPTS', '30')) + click.echo("Tips while parsing:") + click.echo("- Large repositories may take longer to parse") + click.echo("- You can check status later using 'potpie projects'") if not os.path.exists(repo): click.secho("Invalid repository path", fg="red", bold=True) return repo = os.path.abspath(repo) click.secho(f"Starting parsing for repository: {repo}", fg="blue", bold=True) try: project_id = api_wrapper.parse_project(repo, branch) - max_attempts: int = 30 attempt: int = 0 while attempt < max_attempts: status = api_wrapper.parse_status(project_id) logging.info(f"Current status: {status}") if status in ["ready", "error"]: if status == "ready": click.secho("Parsing completed", fg="green", bold=True) else: click.secho("Parsing failed", fg="red", bold=True) break - loading_animation("Parsing in progress") + progress = min(100, (attempt * 100) // max_attempts) + loading_animation(f"Parsing in progress ({progress}%)") attempt += 1 if attempt >= max_attempts: logging.warning("Parsing took too long...") - click.secho("Tips to resolve this...", fg="cyan", bold=True) - click.secho( - "This can be happened due to large repository size, so wait for some time.", - fg="yellow", - ) - click.secho( - "Then manually check the parsing status using 'potpie projects'", - fg="yellow", - ) + click.secho("Parsing is still in progress. Check status later using 'potpie projects'", fg="yellow")
121-178:⚠️ Potential issueFix status code access and improve error handling.
The projects command needs two improvements:
- Fix the status code access.
- Use the
handle_api_errordecorator.Apply this diff:
@cli.command() @click.option("--delete", is_flag=True, help="Enable project deletion mode") +@handle_api_error def projects(delete): """List all projects and optionally delete selected projects""" try: projects = api_wrapper.get_list_of_projects() if not delete: # Standard project listing table_data = [ [project["id"], project["repo_name"], project["status"]] for project in projects ] table = tabulate( table_data, headers=["ID", "Name", "Status"], tablefmt="fancy_grid" ) click.echo(table) else: # Simple delete mode click.echo("Available Projects:") for idx, project in enumerate(projects, 1): click.echo(f"{idx}. {project['repo_name']} (ID: {project['id']})") try: selection = click.prompt( "Enter the number of the project to delete", type=int ) if 1 <= selection <= len(projects): selected_project = projects[selection - 1] selected_project_id = selected_project["id"] confirm = click.confirm( f"Delete {selected_project['repo_name']} with (ID: {selected_project_id})?" ) if confirm: status_code = api_wrapper.delete_project(selected_project_id) if status_code == 200: click.echo(f"Project {selected_project['repo_name']}") click.echo( f"ID: {selected_project_id}) deleted successfully." ) else: click.echo( - f"Failed to delete project. Status code: {status_code.status_code}" + f"Failed to delete project. Status code: {status_code}" ) else: click.echo("Invalid project selection.") except ValueError: click.echo("Invalid input. Please enter a valid project number.")
309-361: 🛠️ Refactor suggestionAdd timeout and improve conversation state management.
The message command needs several improvements:
- Add timeout for agent interaction.
- Improve conversation state management.
- Use the
handle_api_errordecorator.Apply this diff:
+class ConversationContext: + def __init__(self): + self.conversation_id = None + self.title = None + self.status = None + + async def __aenter__(self): + try: + conversations = api_wrapper.get_conversation() + # ... selection logic ... + self.conversation_id = selected_conversation_id + return self + except Exception as e: + logging.error("Failed to initialize conversation: %s", e) + raise + + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass @conversation.command() +@handle_api_error def message(): """Communicate with the agent""" asyncio.run(_message()) async def _message(): """Actual async function for message handling""" - conversation_id: str = "" - try: - conversations: dict = api_wrapper.get_conversation() - # ... conversation selection code ... - except Exception as e: - logging.error("An unexpected error occurred: %s", e) + async with ConversationContext() as ctx: + while True: + try: + user_input: str = input("You: ") - while True: - try: - user_input: str = input("You: ") + if user_input.lower() in ["exit", "quit"]: + click.echo("Exiting chat session.") + break - if user_input.lower() in ["exit", "quit"]: - click.echo("Exiting chat session.") - break - - async for message in api_wrapper.interact_with_agent( - conversation_id=conversation_id, content=user_input - ): - click.echo(message, nl=False) + try: + async with asyncio.timeout(30): # 30 seconds timeout + async for message in api_wrapper.interact_with_agent( + conversation_id=ctx.conversation_id, + content=user_input + ): + click.echo(message, nl=False) + except asyncio.TimeoutError: + click.secho("\nAgent response timed out", fg="red") - print("\n") + print("\n") - except KeyboardInterrupt: - print("\nExiting chat session.") - break + except KeyboardInterrupt: + print("\nExiting chat session.") + break
🧹 Nitpick comments (2)
cli/src/potpie/main.py (2)
33-41: Improve loading animation flexibility.The loading animation has a fixed 5-second duration which might not align with the actual operation time. Consider making it more dynamic:
-def loading_animation(message: str): - """Display loading animation for five seconds""" - spinner = itertools.cycle(["⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷"]) - start_time = time.time() - while (time.time() - start_time) < 5: - sys.stdout.write(f"\r{message} {next(spinner)}") - sys.stdout.flush() - time.sleep(0.1) - sys.stdout.write("\r\033[K") +def loading_animation(message: str, duration: Optional[float] = None, check_condition: Optional[Callable[[], bool]] = None): + """Display loading animation until duration expires or condition is met + + Args: + message: Message to display with the spinner + duration: Optional maximum duration in seconds + check_condition: Optional callback to check if operation is complete + """ + spinner = itertools.cycle(["⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷"]) + start_time = time.time() + while True: + if duration and (time.time() - start_time) >= duration: + break + if check_condition and check_condition(): + break + sys.stdout.write(f"\r{message} {next(spinner)}") + sys.stdout.flush() + time.sleep(0.1) + sys.stdout.write("\r\033[K")
197-282: Consolidate error handling and improve code structure.The create command has multiple try-except blocks that could be consolidated into a single block. Also, consider extracting project and agent selection into separate functions.
+def select_project(api_wrapper): + """Helper function to select a project""" + projects = api_wrapper.get_list_of_projects() + for idx, project in enumerate(projects, 1): + click.echo(f"{idx}. {project['repo_name']} (ID: {project['id']})") + selection = click.prompt( + "Enter the number of the project to start conversation with", type=int + ) + if 1 <= selection <= len(projects): + selected_project = projects[selection - 1] + if click.confirm(f"Start conversation with {selected_project['repo_name']}?"): + return selected_project["id"] + return None + +def select_agent(api_wrapper): + """Helper function to select an agent""" + agents = api_wrapper.available_agents(system_agent=True) + for idx, agent in enumerate(agents, 1): + click.echo(f"{idx}. {agent['name']} (ID: {agent['id']})") + selection = click.prompt( + "Enter the number of the agent to start conversation with", type=int + ) + if 1 <= selection <= len(agents): + selected_agent = agents[selection - 1] + if click.confirm(f"Choose {selected_agent['name']} agent?"): + return selected_agent["id"] + return None @conversation.command() @click.argument("title") @click.option("--max-length", default=100, help="Maximum title length") @handle_api_error def create(title, max_length): """Create a new conversation""" if not title.strip(): click.secho("Title cannot be empty", fg="red") return if len(title) > max_length: click.secho(f"Title exceeds maximum length of {max_length} characters", fg="red") return - project_ids = None - agent_id = None - - try: - projects = api_wrapper.get_list_of_projects() - # ... project selection code ... - except Exception as e: - logging.error("An unexpected error occurred: %s", e) - - try: - agents = api_wrapper.available_agents(system_agent=True) - # ... agent selection code ... - except Exception as e: - logging.error("An unexpected error occurred: %s", e) + project_id = select_project(api_wrapper) + if not project_id: + click.secho("Project selection failed", fg="red") + return - try: - conversation = api_wrapper.create_conversation( - title=title, - project_id_list=[project_ids], - agent_id_list=[agent_id], - ) - click.secho("Conversation created successfully.", fg="green", bold=True) - print(f"Conversation {conversation}") - except Exception as e: - logging.error("An unexpected error occurred: %s", e) + agent_id = select_agent(api_wrapper) + if not agent_id: + click.secho("Agent selection failed", fg="red") + return + + conversation = api_wrapper.create_conversation( + title=title, + project_id_list=[project_id], + agent_id_list=[agent_id], + ) + click.secho("Conversation created successfully.", fg="green", bold=True) + click.echo(f"Conversation {conversation}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cli/src/potpie/main.py(1 hunks)cli/src/potpie/server_manager.py(1 hunks)cli/src/potpie/utility.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
cli/src/potpie/main.py (1)
Learnt from: DeepeshKalura
PR: potpie-ai/potpie#233
File: cli/src/potpie/main.py:186-268
Timestamp: 2025-02-15T08:31:25.708Z
Learning: Use decorators for handling common error patterns in CLI commands to reduce code duplication and improve maintainability, especially for API error handling in Click commands.
🪛 Ruff (0.8.2)
cli/src/potpie/utility.py
45-45: Use capitalized environment variable DEFAULTUSERNAME instead of defaultUsername
(SIM112)
cli/src/potpie/server_manager.py
202-202: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
228-228: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
232-234: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
266-266: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
293-294: Use a single if statement instead of nested if statements
(SIM102)
304-304: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
335-335: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
360-360: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
368-368: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (10)
cli/src/potpie/utility.py (6)
27-30: LGTM!The property is well-implemented with proper documentation.
32-35: LGTM!The property is well-implemented with proper documentation.
37-40: LGTM!The function correctly retrieves the base URL from environment variables with a sensible default value.
42-45: Use uppercase for environment variable names.The environment variable
defaultUsernamedoesn't follow the conventional uppercase naming pattern.Apply this diff:
@staticmethod def get_user_id() -> str: """Retrieve the default username from environment variables.""" - return os.getenv("defaultUsername", "defaultuser") + return os.getenv("POTPIE_DEFAULT_USERNAME", "defaultuser")🧰 Tools
🪛 Ruff (0.8.2)
45-45: Use capitalized environment variable
DEFAULTUSERNAMEinstead ofdefaultUsername(SIM112)
47-62: Improve file creation security.The current file creation method might be vulnerable to race conditions and doesn't set proper file permissions.
Apply this diff:
def _create_logs_files(self, filename) -> str: """Create log files for server communications.""" if os.name == "nt": log_dir = os.path.join(Path(os.getenv("LOCALAPPDATA")), "potpie/logs") else: log_dir = os.path.join(Path.home(), ".local/share/potpie/logs") Path(log_dir).mkdir(parents=True, exist_ok=True) log_file = os.path.join(log_dir, filename) if not os.path.exists(log_file): - with open(log_file, "w", encoding="utf-8") as _: - pass + import stat + flags = os.O_CREAT | os.O_WRONLY + mode = stat.S_IRUSR | stat.S_IWUSR + try: + fd = os.open(log_file, flags, mode) + os.close(fd) + except OSError as e: + logging.error(f"Failed to create log file: {e}") + raise return log_file
64-76: LGTM!The function correctly handles PID file path creation with proper logging and OS-specific paths.
cli/src/potpie/main.py (2)
44-46: LGTM!The main CLI entry point is well-structured with a clear docstring.
180-183: LGTM!The conversation command group is well-structured with a clear docstring.
cli/src/potpie/server_manager.py (2)
87-95: LGTM!The environment check is well-implemented with appropriate logging.
97-119: LGTM!The Docker installation check is well-implemented with proper error handling and logging.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
cli/test/test_server_manager.py (1)
185-327: 🛠️ Refactor suggestionAdd cleanup and resource management to server tests.
The server tests need better resource management:
- Add cleanup fixtures
- Handle temporary files
- Mock system resources
+@pytest.fixture(autouse=True) +def cleanup_files(): + """Clean up temporary files after each test""" + yield + for file in ["poitre.pid", "server.log", "celery.log"]: + if os.path.exists(file): + os.remove(file) +@pytest.fixture +def mock_resources(monkeypatch): + """Mock system resources""" + def mock_memory_info(): + return type('obj', (object,), {'available': 1024 * 1024 * 1024}) + + def mock_cpu_percent(): + return 50.0 + + monkeypatch.setattr('psutil.virtual_memory', mock_memory_info) + monkeypatch.setattr('psutil.cpu_percent', mock_cpu_percent) + return mock_memory_info, mock_cpu_percent @pytest.mark.parametrize( "scenario, expected_exception", [ ("server_running", None), ("server_already_running", StartServerError), ("docker_failure", StartServerError), ("postgres_failure", StartServerError), ("migration_failure", StartServerError), + ("insufficient_memory", StartServerError), + ("high_cpu_usage", StartServerError), ], ) -def test_start_server(server_manager, monkeypatch, scenario, expected_exception): +def test_start_server(server_manager, monkeypatch, mock_resources, scenario, expected_exception):🧰 Tools
🪛 Ruff (0.8.2)
319-319: Comparison to
Noneshould becond is not NoneReplace with
cond is not None(E711)
cli/src/potpie/server_manager.py (1)
412-426: 🛠️ Refactor suggestionImprove shutdown signal handling.
The shutdown signal handler needs improvements:
- Handle more signal types
- Add timeout for graceful shutdown
- Force kill if graceful shutdown fails
+ def _force_kill(self, timeout: int = 5): + """Force kill processes if graceful shutdown fails""" + try: + with open(self.pid_file, "r") as f: + for pid in f.read().strip().split(): + try: + os.kill(int(pid), signal.SIGKILL) + except ProcessLookupError: + pass + except Exception as e: + logging.error(f"Force kill failed: {e}") + def handle_shutdown(self, signum: int, frame: Optional[FrameType]) -> NoReturn: - """Handle shutdown signal, - - Args: - signum: Signal number - - note: we can also aff the frames - - """ + """Handle shutdown signal gracefully with timeout""" logging.info( "Received shutdown signal (%s %s), stopping server...", signum, frame ) - self.stop_server() + try: + with asyncio.timeout(10): # 10 second timeout for graceful shutdown + self.stop_server() + except (asyncio.TimeoutError, Exception) as e: + logging.error(f"Graceful shutdown failed: {e}") + self._force_kill() sys.exit(0)
🧹 Nitpick comments (11)
cli/src/potpie/main.py (5)
26-30: Add error handling for environment initialization.The module-level initialization of
ServerManagerandApiWrappercould fail if required environment variables aren't set. Consider wrapping the initialization in a function that validates the environment.load_dotenv(find_dotenv(), override=True) +def initialize_managers(): + """Initialize server and API managers with environment validation""" + if not os.getenv("ENV"): + raise click.ClickException("ENV variable not set") + return ServerManager(), ApiWrapper() -server_manager = ServerManager() -api_wrapper = ApiWrapper() +server_manager, api_wrapper = initialize_managers()
33-45: Enhance error handling decorator.The current error handling could be more informative and specific:
- Add context about which command failed
- Include more specific exception types
- Return consistent exit codes
def handle_error(func): """Decorator for handling API errors""" def wrapper(*args, **kwargs): try: return func(*args, **kwargs) except requests.RequestException as e: - logging.error("Network error occurred: %s", e) + logging.error("Network error in '%s': %s", func.__name__, e) + raise click.ClickException(f"Network error: {e}") except Exception as e: - logging.error("An unexpected error occurred: %s", e) + logging.error("Error in '%s': %s", func.__name__, e) + raise click.ClickException(str(e)) return wrapper
47-56: Improve loading animation robustness.The loading animation has several limitations:
- Fixed 5-second duration might not match actual operation time
- No cleanup on exceptions
- Direct stdout usage might not work in all environments
-def loading_animation(message: str): +def loading_animation(message: str, duration: float = 5): """Display loading animation for five seconds""" spinner = itertools.cycle(["⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷"]) start_time = time.time() - while (time.time() - start_time) < 5: - sys.stdout.write(f"\r{message} {next(spinner)}") - sys.stdout.flush() - time.sleep(0.1) - sys.stdout.write("\r\033[K") + try: + while (time.time() - start_time) < duration: + click.echo(f"\r{message} {next(spinner)}", nl=False) + time.sleep(0.1) + finally: + click.echo("\r\033[K", nl=False)
83-123: Improve parse command robustness.The parse command has several limitations:
- Hard-coded max_attempts value
- Fixed polling interval
- No progress percentage
+@click.option( + "--timeout", + default=150, + help="Maximum time to wait for parsing (seconds)", +) +@click.option( + "--poll-interval", + default=5, + help="Time between status checks (seconds)", +) -def parse(repo, branch): +def parse(repo, branch, timeout, poll_interval): """Parse a Local repository currently""" - max_attempts: int = 30 - for _ in range(max_attempts): + start_time = time.time() + while (time.time() - start_time) < timeout: status = api_wrapper.parse_status(project_id) logging.info("Current status: %s", status) + progress = min(100, int((time.time() - start_time) * 100 / timeout)) + click.echo(f"\rParsing progress: {progress}%", nl=False)
125-175: Add pagination to projects command.The projects listing could be overwhelming for large numbers of projects.
@cli.command() @click.option("--delete", is_flag=True, help="Enable project deletion mode") +@click.option("--page", default=1, help="Page number") +@click.option("--per-page", default=10, help="Items per page") -def projects(delete): +def projects(delete, page, per_page): """List all projects and optionally delete selected projects""" projects = api_wrapper.get_list_of_projects() + start_idx = (page - 1) * per_page + end_idx = start_idx + per_page + paginated_projects = projects[start_idx:end_idx] if not delete: table_data = [ [project["id"], project["repo_name"], project["status"]] - for project in projects + for project in paginated_projects ] table = tabulate( table_data, headers=["ID", "Name", "Status"], tablefmt="fancy_grid" ) click.echo(table) + total_pages = (len(projects) + per_page - 1) // per_page + click.echo(f"\nPage {page}/{total_pages} (Total items: {len(projects)})")cli/test/test_server_manager.py (4)
21-33: Enhance environment check test coverage.Consider adding test cases for:
- Missing ENV variable
- Empty ENV value
- Case sensitivity in ENV values
@pytest.mark.parametrize( "env_var, expected", [ ("custom", False), ("default", False), ("production", False), ("development", True), + (None, False), # Missing ENV + ("", False), # Empty ENV + ("DEVELOPMENT", False), # Case sensitive + (" development ", False), # Whitespace ], )
35-100: Simplify Docker test data and add edge cases.The Docker test uses overly complex mock data. Consider:
- Simplifying the test data
- Adding edge cases for container health checks
- Testing timeout scenarios
@pytest.mark.parametrize( "is_docker_installed, docker_compose_up, docker_ps_output, expected_exception", [ (False, None, None, DockerError), (True, DockerError, None, DockerError), - ( - True, - 0, - """{"State": "\\"excited\\""}\n{"State": "\\"excited\\""}\n{"State": "excited"}\n""", - DockerError, - ), + # Simplified test data + (True, 0, '[{"State": "starting"}, {"State": "starting"}]', DockerError), + (True, 0, '[{"State": "running"}, {"State": "exited"}]', DockerError), + (True, 0, '[{"State": "running"}, {"State": "running"}]', None), + # New edge cases + (True, 0, '[]', DockerError), # No containers + (True, 0, 'invalid json', DockerError), # Invalid JSON ], )
102-136: Add edge cases to PostgreSQL tests.The PostgreSQL test could cover more scenarios:
- Connection timeout
- Authentication failure
- Invalid host/port
@pytest.mark.parametrize( "scenario, returncode, stdout, should_raise", [ ("success", 0, "PostgreSQL is running and accepting connections", False), ("failure", 1, "PostgreSQL is not responding", True), ("exception", None, None, True), + ("timeout", 1, "connection timeout", True), + ("auth_error", 1, "authentication failed", True), + ("invalid_host", 1, "could not translate host name", True), ], )🧰 Tools
🪛 Ruff (0.8.2)
135-135: Avoid equality comparisons to
True; useif server_manager.check_postgres():for truth checksReplace with
server_manager.check_postgres()(E712)
138-183: Enhance migration test coverage.Add test cases for:
- Migration version conflicts
- Database schema issues
- Permission errors
@pytest.mark.parametrize( "scenario, isalembic, returncode, stdout, should_raise", [ ("alembic_not_found", 0, False, None, True), ("failure", True, 1, "PostgreSQL is not responding", True), ("exception", True, None, None, True), ("success", True, 0, "PostgreSQL is running and accepting connections", False), + ("version_conflict", True, 1, "Multiple head revisions", True), + ("schema_error", True, 1, "Table already exists", True), + ("permission_error", True, 1, "Permission denied", True), ], )cli/src/potpie/server_manager.py (1)
77-86: Improve resource cleanup in ServerManager initialization.The ServerManager should implement context management for better resource cleanup.
class ServerManager: """A class to manage the Potpie development server.""" + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + """Ensure cleanup of resources""" + try: + if os.path.exists(self.pid_file): + self.stop_server() + except Exception as e: + logging.warning(f"Cleanup failed: {e}") + def __init__(self): utility: Utility = Utility() self.pid_file = Utility.create_path_of_pid_file() self.server_log = utility.server_log_file self.celery_log = utility.celery_log_filecli/example/example.md (1)
1-102: Improve markdown documentation formatting.Several improvements needed:
- Remove trailing colons from headings
- Add language specifiers to code blocks
- Add table of contents
- Add version information
# Potpie Project Usage Summary +## Table of Contents + +- [Overview](#overview) +- [Installation](#installation) +- [Starting the Server](#starting-the-server) +- [Listing Projects](#listing-projects) +- [Parsing a Repository](#parsing-a-repository) +- [Starting a Conversation](#starting-a-conversation) +- [Stopping the Server](#stopping-the-server) +- [Summary](#summary) + +## Version + +This documentation is for Potpie CLI version 0.0.1. + ## Overview -### Logs: +### Logs -### Output: +### Output ```console ╒══════╤════════╤══════════╕-### Parsing Status Updates:
+### Parsing Status Updates-### Available Agents:
+### Available Agents-### Logs:
+### Logs<details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 18-18: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 30-30: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 31-31: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 43-43: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 49-49: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 64-64: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 78-78: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 88-88: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e90e76ff894beb3b8fec567203b636aceb25bcd7 and 123c42b6aec1ef50bae987d35342e7dfa19f7c31. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `cli/example/example.md` (1 hunks) * `cli/src/potpie/main.py` (1 hunks) * `cli/src/potpie/server_manager.py` (1 hunks) * `cli/test/test_server_manager.py` (1 hunks) * `requirements.txt` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * requirements.txt </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (1)</summary> <details> <summary>cli/src/potpie/main.py (1)</summary>Learnt from: DeepeshKalura
PR: #233
File: cli/src/potpie/main.py:186-268
Timestamp: 2025-02-15T08:31:25.708Z
Learning: Use decorators for handling common error patterns in CLI commands to reduce code duplication and improve maintainability, especially for API error handling in Click commands.</details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>cli/example/example.md</summary> 18-18: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 30-30: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 31-31: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 43-43: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 49-49: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 64-64: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 78-78: Fenced code blocks should have a language specified null (MD040, fenced-code-language) --- 88-88: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) </details> </details> <details> <summary>🪛 Ruff (0.8.2)</summary> <details> <summary>cli/src/potpie/server_manager.py</summary> 260-260: Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling (B904) --- 264-266: Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling (B904) --- 298-298: Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling (B904) --- 325-326: Use a single `if` statement instead of nested `if` statements (SIM102) --- 336-336: Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling (B904) --- 367-367: Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling (B904) --- 392-392: Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling (B904) --- 402-402: Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling (B904) </details> <details> <summary>cli/test/test_server_manager.py</summary> 135-135: Avoid equality comparisons to `True`; use `if server_manager.check_postgres():` for truth checks Replace with `server_manager.check_postgres()` (E712) --- 319-319: Comparison to `None` should be `cond is not None` Replace with `cond is not None` (E711) </details> </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
@dhirenmathur I thoroughly checked the CLI, and I think it's good to go. DocumentationThis docs helps developer to setup cli tool asap Setting Up the Development Environment1. Create a Virtual EnvironmentBefore installing dependencies, create a virtual environment: python -m venv venvActivate the virtual environment:
2. Install DependenciesInstall the package in editable mode along with development dependencies: pip install -e .[dev]3. Build the PackageOnce dependencies are installed, build the package: python -m build4. Install the CLI tool in a Core Package PlaceAfter building the package, install it: pip install cli/dist/python-package5. Using the Package in Docs and ExamplesYou can now import and use the package in your documentation and examples: potpie start |
added pytest_cach and poitre.pid in the .gitignore
added tabulate and pytest in the requirements.txt
Currently, there is an internal error in the neo4j while in the knowledge graph. If I use return like we did then I will get error in the messaging. I think I don't handle it perfectly with the stream because the message is not working.
Docs
Poitre CLI Tool
A command-line interface (CLI) tool to manage and interact with the Poitre application, including managing servers, parsing repositories, listing projects, and interacting with conversations.
Usage
The CLI provides multiple commands for managing the application. Below is a list of commands and their functionalities.
Global Help
Displays the list of available commands and their descriptions.
Server Commands
Start the Server
Stop the Server
Project Management
Parse a Repository
repository_path: Path to the repository on the local machine.--branch: (Optional) Name of the branch to parse (default:main).List Projects
Delete a Project
Conversation Management
List Conversations
Create a Conversation
title: Title of the conversation.Message an Agent
Logging
celery.log: Logs from the Celery worker.celery_error.log: Error logs from the Celery worker.server.log: Logs from the server.server_error.log: Error logs from the server.Improvement
Logging and poitre.pid file needs improvement.
Summary by CodeRabbit