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

Feature/change port #88

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

Conversation

aaryan182
Copy link

Related Issue
Fixes #56

Description of Changes
This PR implements proper port configuration handling across the Preswald application ensuring consistent behavior between CLI arguments, configuration files, and default values.

In main.py file
=> Added proper port priority logic in start_server function
=> Implemented environment variable setting for frontend communication
=> Added clear logging messages for port configuration
=> fixed linter errors

In vite.config.js in frontend
=> Updated proxy configuration to use environment variable
=> Added fallback to default port
=> Made proxy configuration more robust with changeOrigin

In cli.py file
=> Updated help text to accurately describe port configuration behavior
=> Added clear documentation about port priority
=> Improved command descriptions

Port Priority Implementation

The changes establish a clear port priority order:

  1. Command-line argument (--port)
  2. Configuration file (preswald.toml)
  3. Default value (8501)

Tested Locally CLI:

=> Installed pytest and pytest-mock
=> Created a test_port_config.py file
=> test_port_priority_cli_argument: Verifies that CLI argument (--port) takes highest precedence
=> test_port_priority_config_file: Verifies that config file port is used when no CLI override
=> test_port_priority_default: Verifies fallback to default port (8501)
=> test_invalid_config_file: Verifies graceful handling of invalid config files

Type of Change

  • [✅ ] New feature (non-breaking change which adds functionality)

Testing
image

Checklist

  • [ ✅] My code follows the style guidelines of this project
  • [ ✅] I have performed a self-review of my own code
  • [✅ ] I have commented my code, particularly in hard-to-understand areas

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.

Allow to change port from default 8501
1 participant