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

Improve setup script with better error handling and feedback #215

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

IronJam11
Copy link

Summary

This PR enhances the project setup script with proper error handling, dependency checking, and user feedback. The improvements make the script more robust and user-friendly while fixing the incorrect virtual environment deactivation command.

Changes Made

  • Added error handling with set -e
  • Created error and success message functions with color coding
  • Added dependency checks for python3, git, and wget
  • Fixed virtual environment deactivation (removed incorrect source deactivate)
  • Added progress messages for better user feedback
  • Improved variable quoting for better security
  • Added comments for better maintainability

Testing Done

  • Tested script on a clean environment
  • Verified error handling by simulating various failure conditions:
    • Missing dependencies
    • Network issues during downloads
    • Permission issues
  • Confirmed proper virtual environment activation and deactivation
  • Tested on bash 5.0+ environment

Issues

Fixes #214

Checklist

  • Code follows project style guidelines
  • Added comments for better maintainability
  • Tested on a clean environment
  • Error handling for all critical operations
  • User feedback for all major steps

@Aditya062003
Copy link
Contributor

Good work! But, iirc there were some issues about setup script failing in some scenarios, it would be great if you could check that, and resolves those here itself.

@IronJam11
Copy link
Author

So turns out the following changes helped:

  • Increasing the git buffer size
  • Using HTTPS over HTTP/2
Screenshot 2025-02-18 at 2 08 46 AM

@Aditya062003
Copy link
Contributor

Great! I noticed both of these PRs are addressing similar issues: (#188). It would be best if both of you collaborated to integrate these fixes into a single PR. You can work together to refine the solution and submit a combined PR. Let me know if you need any help! 🚀

@IronJam11
Copy link
Author

So basically we have made a few improvments after discussing it together :-

  • Added debugging
  • Resolved the problem with installing the thinc and torch packages
  • Added documentation
  • Removed deactivate command so that the user can have the server running on script execution (tested)
  • Added colours for representing errors and successes correspondingly
  • Add command in script to perform the setup automatically (extraction,running server)

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.

Setup Script Needs Error Handling and Better User Experience
2 participants