Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/package - part 1 #161

Merged
merged 38 commits into from
Feb 15, 2025
Merged

Feat/package - part 1 #161

merged 38 commits into from
Feb 15, 2025

Conversation

srijanpatel
Copy link
Collaborator

@srijanpatel srijanpatel commented Feb 15, 2025

PySpur PyPI package - Part 1

This PR prepares the codebase to release pyspur python package on PyPI.
In production docker mode, the frontend code is served as static files by the FastAPI server that also serves the APIs.
The docker-compose for this no longer ships with an nginx and f/e service.
Furthermore, the release GitHub action has been updated to not ship frontend image any longer.

./backend/app directory has been renamed to ./backend/pyspur to prep for the python package release in a follow up PR for this.


This pull request includes significant updates to the development environment, Docker configurations, and backend structure. The most important changes include modifications to the .devcontainer settings, updates to Docker configurations, and refactoring of backend files and migrations.

Development Environment Updates:

  • .devcontainer/devcontainer.json: Added new extensions charliermarsh.ruff and tamasfe.even-better-toml, enabled Python linting and formatting with ruff, and added TypeScript settings. [1] [2]

Docker Configuration Updates:

  • .dockerignore: Added a comprehensive list of files and directories to ignore, including version control, dependencies, build outputs, and more.
  • Dockerfile.backend: Consolidated Dockerfile for backend, including stages for development, frontend build, and production.
  • Dockerfile.frontend: Updated and renamed frontend/Dockerfile to Dockerfile.frontend, adjusting paths and build instructions. [1] [2]

Backend Refactoring:

  • .env.example: Changed NGINX_PORT to PYSPUR_PORT for clarity in application port configuration.
  • backend/app/api/main.py: Removed the entire file, indicating a major refactor or migration of API routes and initialization.
  • backend/app/models/management/alembic/versions/000_init_db.py and backend/app/models/management/alembic/versions/002_add_knowledge_base_model.py: Removed old Alembic migration scripts, suggesting a database schema overhaul. [1] [2]

Workflow Adjustments:


Important

Prepare codebase for PyPI package release by restructuring backend, updating Docker configurations, and modifying development environment.

  • Backend Restructuring:
    • Renamed ./backend/app to ./backend/pyspur for PyPI package preparation.
    • Removed backend/app/api/main.py and refactored to backend/pyspur/api/main.py.
    • Updated Alembic migration scripts in backend/pyspur/models/management/alembic/versions/.
  • Docker Configuration:
    • Consolidated Dockerfiles: Dockerfile.backend and Dockerfile.frontend for backend and frontend builds.
    • Updated docker-compose.yml, docker-compose.prod.yml, and docker-compose.staging.yml to reflect new Dockerfile paths and service configurations.
    • Removed Nginx and frontend service from production docker-compose.
  • Development Environment:
    • Updated .devcontainer/devcontainer.json with new extensions and settings.
    • Added pyproject.toml for project configuration and dependencies.
  • Workflow Adjustments:
    • Modified .github/workflows/release.yml to remove frontend image build and focus on backend.
  • Miscellaneous:
    • Updated nginx/conf.d/default.conf to adjust API proxy settings.
    • Updated frontend/next.config.ts to set output to 'export'.

This description was created by Ellipsis for fd09c47. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to b5ff66b in 4 minutes and 36 seconds

More details
  • Looked at 8154 lines of code in 170 files
  • Skipped 2 files when reviewing.
  • Skipped posting 23 drafted comments based on config settings.
1. backend/pyspur/nodes/registry.py:64
  • Draft comment:
    Review the relative module path formatting when registering nodes; prefixing with a dot ('.') may affect imports when packaging.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. backend/pyproject.toml:21
  • Draft comment:
    Double-check the dependency 'backend==0.2.4.1'; if this is an internal package, ensure it’s correctly referenced and not conflicting with external packages.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to double-check a dependency, which violates the rule against asking for confirmation or verification. It also involves dependency management, which is not allowed to be commented on unless it's a specific version mismatch issue.
3. .github/workflows/release.yml:41
  • Draft comment:
    Ensure Dockerfile/backend location is updated to reflect new folder structure (context: '.' and file: './Dockerfile.backend').
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
4. Dockerfile.backend:30
  • Draft comment:
    Verify that the frontend build output directory '/pyspur/frontend/out/' matches the actual build output.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to verify something, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
5. Dockerfile.frontend:3
  • Draft comment:
    Check that 'COPY frontend/package*.json ./' correctly locates package files in the new structure.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to check if a specific command correctly locates package files in a new structure. This falls under asking the author to double-check or ensure something, which is against the rules. Therefore, this comment should be removed.
6. backend/entrypoint.sh:10
  • Draft comment:
    The entrypoint now reflects the new package structure; confirm that 'uvicorn "pyspur.api.main:app"' starts the server as expected.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to confirm that a specific command starts the server as expected. This falls under asking the author to confirm their intention or to ensure behavior, which is against the rules.
7. backend/pyproject.toml:21
  • Draft comment:
    Ensure the dependency 'backend==0.2.4.1' does not conflict with the local module name; review dependency versions.
  • Reason this comment was not posted:
    Marked as duplicate.
8. backend/pyspur/api/main.py:75
  • Draft comment:
    Consider replacing print statements with proper logging for production use.
  • Reason this comment was not posted:
    Marked as duplicate.
9. backend/pyspur/api/workflow_run.py:181
  • Draft comment:
    Revisit session management in background tasks; using 'with next(get_db()) as session:' may need more robust handling.
  • Reason this comment was not posted:
    Comment was on unchanged code.
10. backend/pyspur/integrations/slack/client.py:35
  • Draft comment:
    Use proper logging instead of print statements for error handling in SlackClient.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. backend/pyspur/models/management/alembic/env.py:27
  • Draft comment:
    Ensure Alembic logging and database URL setup meet production standards.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. backend/pyspur/models/management/alembic/versions/000_init_db.py:27
  • Draft comment:
    Verify that computed columns (e.g., 'DS' || _intid) are supported by your target database.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to verify something, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code. Therefore, it should be removed.
13. backend/pyspur/models/management/alembic/versions/002_add_knowledge_base_model.py:20
  • Draft comment:
    Migration for 'knowledge_bases' looks standard; confirm it aligns with your intended schema.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to confirm their intention regarding the migration for 'knowledge_bases'. It does not provide a specific suggestion or point out a specific issue, which violates the rules.
14. backend/pyspur/models/management/alembic/versions/003_split_knowledge_base_into_dc_and_vi.py:77
  • Draft comment:
    Ensure data migration considerations are handled when splitting 'knowledge_bases' into two tables.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to ensure that data migration considerations are handled, which is a form of asking them to double-check their work. This violates the rule against asking the author to ensure something is done. The comment does not provide a specific suggestion or point out a specific issue with the code.
15. backend/pyspur/models/management/alembic/versions/004_add_progress_status.py:23
  • Draft comment:
    The progress status migration appears appropriate; double-check if any additional indexes are needed.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to double-check something, which violates the rule against asking the author to confirm or double-check things. It does not provide a specific suggestion or point out a specific issue.
16. backend/pyspur/nodes/factory.py:67
  • Draft comment:
    Consider adding error handling around config model instantiation in case validation fails.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
17. backend/pyspur/nodes/registry.py:60
  • Draft comment:
    Verification: Module path adjustment to remove 'pyspur.' prefix is appropriate for consistent imports.
  • Reason this comment was not posted:
    Marked as duplicate.
18. backend/pyspur/schemas/node_type_schemas.py:11
  • Draft comment:
    NodeTypeSchema properties correctly use import_module with package 'pyspur'; ensure attributes like display_name and logo are set in node classes.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the author to ensure that certain attributes are set, which is similar to asking them to double-check or verify something. This violates the rule against asking the author to ensure behavior is intended or tested.
19. docker-compose.prod.yml:1
  • Draft comment:
    Production Compose now omits the frontend service; confirm intentional removal and validate volume/environment configurations.
  • Reason this comment was not posted:
    Comment was on unchanged code.
20. docker-compose.staging.yml:15
  • Draft comment:
    Staging Compose configuration appears correct; ensure read-only mounts and volume paths match expectations.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure that certain configurations match expectations, which violates the rule against asking the author to confirm or ensure things. It doesn't provide a specific suggestion or point out a specific issue.
21. docker-compose.yml:2
  • Draft comment:
    Development Compose combines nginx, backend, and frontend; verify port mappings and dependency conditions to avoid conflicts.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
22. frontend/next.config.ts:1
  • Draft comment:
    Next.js config with 'output: export' is suited for static site exports; ensure this aligns with your deployment strategy.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure that their deployment strategy aligns with the configuration change. It is not making a specific suggestion or pointing out a specific issue with the code. It violates the rule against asking the author to ensure behavior is intended.
23. nginx/conf.d/default.conf:6
  • Draft comment:
    Nginx config correctly proxies API and frontend requests; verify that 'proxy_pass http://frontend:3000;' is appropriate for your environment.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_Xh5NtL04aKsFahXl


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@srijanpatel srijanpatel merged commit 9cd73ce into main Feb 15, 2025
1 check passed
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on fd09c47 in 2 minutes and 7 seconds

More details
  • Looked at 49 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. backend/pyspur/api/main.py:75
  • Draft comment:
    Removed debug print for serving the frontend. If you still need diagnostic info, consider using the logging module instead of print statements in production.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
    While the comment is accurate about what changed, it's more of an informative comment explaining the change rather than requesting a specific code change. The print statements were already removed, and the suggestion about logging is more of a general best practice than a specific issue that needs fixing. The comment doesn't identify a problem that needs to be addressed.
    The suggestion about using logging could be considered actionable feedback for improving the code quality. Maybe removing debug statements without replacing them with proper logging is a missed opportunity?
    The comment is primarily explaining what was already done (removing prints) and making a general suggestion. It doesn't point out a specific issue that needs fixing, and we don't want to keep purely informative comments.
    Delete the comment because it's primarily informative and doesn't request a specific code change. The print statements were already removed, and the logging suggestion is too general.
2. backend/pyspur/api/main.py:85
  • Draft comment:
    Removed multiple print statements for file/directory checks. If logging is required for debugging, integrate a logger rather than using print.
  • Reason this comment was not posted:
    Marked as duplicate.
3. backend/pyspur/api/main.py:75
  • Draft comment:
    Good cleanup: removed debug print statements. For future debugging, consider using a proper logging framework (e.g., logger.debug) instead of print.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None

Workflow ID: wflow_mzLwQ51SdtUmWQuM


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

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.

1 participant