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

Allow opentrader up to specify a custom port #92

Open
akanoce opened this issue Jan 4, 2025 · 10 comments
Open

Allow opentrader up to specify a custom port #92

akanoce opened this issue Jan 4, 2025 · 10 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@akanoce
Copy link

akanoce commented Jan 4, 2025

Currently, the opentrader up command defaults to using port 8000, with no option to override it. This can cause conflicts if port 8000 is already in use by another service, forcing users to manually stop those services or look for workarounds.

It would be helpful to add an optional --port (or -p) parameter to the opentrader up command. This would allow users to specify a custom port, making the tool more flexible in environments with conflicting services.

@bludnic
Copy link
Member

bludnic commented Jan 5, 2025

@akanoce Express.js was replaced with Fastify in #90

@bludnic bludnic added enhancement New feature or request good first issue Good for newcomers labels Jan 5, 2025
@montymi
Copy link
Contributor

montymi commented Jan 6, 2025

#92 Adding --port flag for daemon start options

Description

Currently, opentrader up only supports the --daemon or -d flags. Adding the ability to customize the port via a --port flag will allow for greater flexibility, especially in environments where port conflicts may arise.

Tasks

  • Update addUpCommand in apps/cli/src/commands/up.ts to handle a --port option, e.g.,
new Option("-p, --port <port>", "Customize port server should attach to").default(8000)
  • include new port option in Options and up() in apps/cli/src/api/up/index.ts
  • pass the port flag to apps/cli/src/api/up/daemon.ts from apps/cli/src/api/up/index.ts in Daemon.create()
  • update hardcoded value in apps/cli/src/daemon-rpc.ts

@bludnic, let me know if I missed anything! I quickly wrote out the tasks for this PR while the daemon package is still fresh in my mind.

@bludnic
Copy link
Member

bludnic commented Jan 6, 2025

@montymi Thanks! You described it perfectly.

@montymi
Copy link
Contributor

montymi commented Jan 6, 2025

Sweet! Taking a look at this now and I'll push my changes later tonight.

@bludnic
Copy link
Member

bludnic commented Jan 6, 2025

@montymi Perhaps you could also add the --hostname option #58. The task is quite similar. I suggest to use localhost (127.0.0.1) by default.

@Charlesnorris509
Copy link

Assign me With this task if it's not completed yet ? @akanoce

@akanoce
Copy link
Author

akanoce commented Jan 7, 2025

@Charlesnorris509 yes feel free to take it, I hadn't time to look into this yet. I think @bludnic can assign you the task

@bludnic
Copy link
Member

bludnic commented Jan 7, 2025

Hey @Charlesnorris509, you can check other tasks with good first issue Good for newcomers label. The current task is already a work in progress by @montymi.

@montymi
Copy link
Contributor

montymi commented Jan 8, 2025

Just pushed my changes in #95

@bludnic
Copy link
Member

bludnic commented Jan 19, 2025

Merged #95. Will be included in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants