added automatic portChange feature if current port is busy #1066

EraKin575 · 2023-11-03T08:51:59Z

Related Issue

Info about Issue or bug

Closes: #1051

Describe the changes you've made

added a automatic port changing feature for test and record mode if current port is busy

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Code style update (formatting, local variables)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Please let us know if any test cases are added

Please describe the tests(if any). Provide instructions how its affecting the coverage.

Describe if there is any unusual behaviour of your code(Write `NA` if there isn't)

A clear and concise description of it.

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.
I have made corresponding changes to the documentation.
My changes generate no new warnings.
I have added tests that prove my fix is effective or that my feature works.
New and existing unit tests pass locally with my changes.

Screenshots (if any)

Original	Updated
original screenshot	updated screenshot

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

sweep-ai · 2023-11-03T08:53:03Z

Apply Sweep Rules to your PR?

Apply: All new business logic should have corresponding unit tests.
Apply: Refactor large functions to be more modular.

nakulbh

Looks good to me

AkashKumar7902 · 2023-11-04T06:11:39Z

What if the next port is in use by some other running service? A while loop with a random function to generate port would be better I believe

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

charankamarapu

Please address the comments

pkg/proxy/proxy.go

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

charankamarapu

Please address the comment

pkg/proxy/proxy.go

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

charankamarapu

Please address the comments, rest looks fine.

charankamarapu · 2023-11-09T15:11:47Z

pkg/proxy/proxy.go

+		for i := 1024; i <= 65535 && attemptsDone < maxAttempts; i++ {
+			if isPortAvailable(uint32(i)) {
+				opt.Port = uint32(i)
+				logger.Info(Emoji+"Port %v is already in use, switching to next available port");


I don't think we need emoji here it is already appended when we initialise logger and include the port number which you are switching to in the log.

charankamarapu · 2023-11-09T15:13:53Z

pkg/proxy/proxy.go

+	}
+
+	if maxAttempts<=attemptsDone{
+		logger.Error(Emoji+"Failed to find an available port to start proxy server")


Remove the emoji.

charankamarapu · 2023-11-09T15:16:46Z

pkg/proxy/proxy.go

+		}
+	}
+
+	if maxAttempts<=attemptsDone{


beautify the statement I guess there must be spaces before and after the <= symbol. There must be some extension in the IDE you can use it and make sure it is indentation is correct.

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

charankamarapu

Please address the comment.

charankamarapu · 2023-11-09T15:34:41Z

pkg/proxy/proxy.go

 	//IPv4
 	localIp4, err := util.GetLocalIPv4()
 	if err != nil {
-		log.Fatalln(Emoji+"Failed to get the local Ip4 address", err)
+		log.Fatalln("Failed to get the local Ip4 address", err)


Why is it removed from here? This is not zap.logger, It is just log. Here we should use emoji.

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

charankamarapu · 2023-11-09T19:40:52Z

pkg/proxy/proxy.go


+    if !isPortAvailable(opt.Port) {
+		logger.Info("Port is already in use, switching to next available port",zap.Uint32("port",opt.Port));


Port is already in use, switching to next available port {port : 16789} this is how the logger looks when you run keploy. It gives a sense that you found that the port is already in use and you are switching to port 16789 which is wrong. You can delete this log or correct it . Deleting this log makes more sense for me.

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

charankamarapu

ship it!

added automatic portChange feature if current port is busy

6a494d4

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

nakulbh reviewed Nov 3, 2023

View reviewed changes

added for for continuous port change if consecutive ports are busy

dcf5cb2

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

EraKin575 force-pushed the dev/portChange branch from 12fd8ab to dcf5cb2 Compare November 4, 2023 17:19

charankamarapu self-requested a review November 5, 2023 07:12

charankamarapu reviewed Nov 7, 2023

View reviewed changes

pkg/proxy/proxy.go Outdated Show resolved Hide resolved

defined a upper and lower bound and maxatempts

767221f

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

EraKin575 force-pushed the dev/portChange branch from d474820 to 767221f Compare November 7, 2023 08:47

EraKin575 requested a review from charankamarapu November 7, 2023 16:01

charankamarapu reviewed Nov 9, 2023

View reviewed changes

pkg/proxy/proxy.go Show resolved Hide resolved

added a case where the system can't find a available port

dd880ea

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

EraKin575 force-pushed the dev/portChange branch from 301602b to dd880ea Compare November 9, 2023 06:15

EraKin575 requested a review from charankamarapu November 9, 2023 11:44

charankamarapu reviewed Nov 9, 2023

View reviewed changes

improved code indentation

4048abb

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

EraKin575 force-pushed the dev/portChange branch from ffd6221 to 4048abb Compare November 9, 2023 15:30

Merge branch 'main' into dev/portChange

4d319d8

EraKin575 requested a review from charankamarapu November 9, 2023 15:31

charankamarapu reviewed Nov 9, 2023

View reviewed changes

fixed emoji

da5e7dd

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

EraKin575 requested a review from charankamarapu November 9, 2023 15:43

EraKin575 added 2 commits November 9, 2023 22:06

fixed logger

8f76a0b

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

removed useless imports

9fcb810

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

charankamarapu reviewed Nov 9, 2023

View reviewed changes

EraKin575 and others added 2 commits November 10, 2023 14:02

removed unnecessary logs

c0103a6

Signed-off-by: EraKin575 <tejaskumar574@gmail.com>

Merge branch 'main' into dev/portChange

85ae66b

charankamarapu approved these changes Nov 10, 2023

View reviewed changes

charankamarapu merged commit 7f8c5a2 into keploy:main Nov 10, 2023
6 checks passed

github-actions bot locked and limited conversation to collaborators Nov 10, 2023

EraKin575 deleted the dev/portChange branch November 10, 2023 12:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added automatic portChange feature if current port is busy #1066

added automatic portChange feature if current port is busy #1066

EraKin575 commented Nov 3, 2023

sweep-ai bot commented Nov 3, 2023

nakulbh left a comment •

edited

Loading

AkashKumar7902 commented Nov 4, 2023

charankamarapu left a comment

charankamarapu left a comment

charankamarapu left a comment

charankamarapu Nov 9, 2023

charankamarapu Nov 9, 2023

charankamarapu Nov 9, 2023

charankamarapu left a comment

charankamarapu Nov 9, 2023

charankamarapu Nov 9, 2023 •

edited

Loading

charankamarapu left a comment


		if !isPortAvailable(opt.Port) {
		logger.Info("Port is already in use, switching to next available port",zap.Uint32("port",opt.Port));

added automatic portChange feature if current port is busy #1066

added automatic portChange feature if current port is busy #1066

Conversation

EraKin575 commented Nov 3, 2023

Related Issue

Describe the changes you've made

Type of change

Please let us know if any test cases are added

Describe if there is any unusual behaviour of your code(Write NA if there isn't)

Checklist:

Screenshots (if any)

sweep-ai bot commented Nov 3, 2023

Apply Sweep Rules to your PR?

nakulbh left a comment • edited Loading

Choose a reason for hiding this comment

AkashKumar7902 commented Nov 4, 2023

charankamarapu left a comment

Choose a reason for hiding this comment

charankamarapu left a comment

Choose a reason for hiding this comment

charankamarapu left a comment

Choose a reason for hiding this comment

charankamarapu Nov 9, 2023

Choose a reason for hiding this comment

charankamarapu Nov 9, 2023

Choose a reason for hiding this comment

charankamarapu Nov 9, 2023

Choose a reason for hiding this comment

charankamarapu left a comment

Choose a reason for hiding this comment

charankamarapu Nov 9, 2023

Choose a reason for hiding this comment

charankamarapu Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

charankamarapu left a comment

Choose a reason for hiding this comment

Describe if there is any unusual behaviour of your code(Write `NA` if there isn't)

nakulbh left a comment •

edited

Loading

charankamarapu Nov 9, 2023 •

edited

Loading