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

Find unused port for codechecker server #3663

Merged
merged 2 commits into from
Apr 28, 2022
Merged

Find unused port for codechecker server #3663

merged 2 commits into from
Apr 28, 2022

Conversation

tal66
Copy link
Contributor

@tal66 tal66 commented Apr 27, 2022

Hi, this closes #3657.
if default port is not available, and the user didn't ask for a specific one - let the os find a port.

@bruntib bruntib added this to the release 6.20.0 milestone Apr 28, 2022
@bruntib bruntib added enhancement 🌟 CLI 💻 Related to the command-line interface, such as the cmd, store, etc. commands new feature 👍 New feature request labels Apr 28, 2022
Copy link
Contributor

@bruntib bruntib left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement, it's a very nice job!
I think, however, that finding a free port shouldn't be an automatic thing even in case of using the default port. If the default port (8001) is not free then it still should be an error. We should only get a free port from os only if the user explicitly provides port 0.

@tal66
Copy link
Contributor Author

tal66 commented Apr 28, 2022

so the user can just use -p 0 in that case. btw, i think you should change the -v option, it is expected to mean verbose in my opinion

@bruntib
Copy link
Contributor

bruntib commented Apr 28, 2022

Oh, sorry, the specification in the original ticket is not accurate. You implemented it according to the specification, but I think we shouldn't consider port 8001 as a special case. We should choose one of these alternatives:

  • If the user provides a port number then we use that. If it's not free then an error message should be printed. If the user doesn't provide any port number then we use 8001 which also can be occupied in which case an error message is printed. If user provides 0 as port number explicitly then then os should find a free one.
  • If the user doesn't provide a port number then CodeChecker should work as if 0 was given. With other words: 0 could be the default port number.

In my previous comment I suggested the first option, because many other CodeChecker commands are assuming this port number. For example CodeChecker store stores to port 8001 by default, so that's why I's vote on the first alternative.

@tal66
Copy link
Contributor Author

tal66 commented Apr 28, 2022

without my commit these are the scenarios:
-p 0 - os finds a port
-p <num> - use num or error
(no port request) - use 8001 or error
which match your first alternative.

@tal66
Copy link
Contributor Author

tal66 commented Apr 28, 2022

could change the error message to say "Server can't be started ... use -p 0 to find a free port automatically", without any code changes.

@bruntib
Copy link
Contributor

bruntib commented Apr 28, 2022

Yes, that message would be a good solution. I've checked the original version of this ticket in an internal ticket system and this task belonged to an old CodeChecker version. Port 0 really works as expected by now, sorry, I haven't checked it before creating this ticket. My bad :(

@tal66
Copy link
Contributor Author

tal66 commented Apr 28, 2022

no problem. I canceled the changes, and just changed the message

Copy link
Contributor

@bruntib bruntib left a comment

Choose a reason for hiding this comment

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

Thank you!

@bruntib bruntib merged commit 01a042c into Ericsson:master Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI 💻 Related to the command-line interface, such as the cmd, store, etc. commands enhancement 🌟 new feature 👍 New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CodeChecker server - implement listening on 'port 0' behavior
2 participants