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

Tweaks for iq #66

Merged
merged 3 commits into from
Jun 23, 2021
Merged

Tweaks for iq #66

merged 3 commits into from
Jun 23, 2021

Conversation

maurycupitt
Copy link
Contributor

Quick tweaks to add host.docker.internal as the default IQ and a type in an error output

cc @bhamail / @DarthHater

@@ -219,7 +219,7 @@ Flags:
-v, -- count Set log level, higher is more verbose
-h, --help help for iq
-a, --iq-application string Specify public application ID for request (required)
-x, --iq-server-url string Specify Nexus IQ Server URL (default "http://localhost:8070")
-x, --iq-server-url string Specify Nexus IQ Server URL (default "http://host.docker.internal:8070")
Copy link
Member

Choose a reason for hiding this comment

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

Wildddd, does this make a pretty noticeable difference?

@@ -45,5 +45,5 @@ func getLogFileMessage() string {
}

return fmt.Sprintf("For more information, check the log file at %s\n"+
"nancy version: %s\n", logFile, buildversion.BuildVersion)
"ahab version: %s\n", logFile, buildversion.BuildVersion)
Copy link
Member

Choose a reason for hiding this comment

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

THANK YOU FOR FIXING WHAT IS LIKELY JEFFRY BEING VERY COPY PASTA MANE (don't even have to git blame this to know my fingers and command+c +v were moving faster than my brain)

Copy link
Member

@DarthHater DarthHater left a comment

Choose a reason for hiding this comment

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

No major objections. It's just an override of a default value, not like someone can't supply something else (and using localhost:8070 is probably mostly limited to us doing local dev more than real world scenarios). Asked for some clarity on what it improves but solely to understand why, @maurycupitt and anyone in SE making this change, I'm gonna assume it makes a non trivial difference :)

@@ -219,7 +219,7 @@ Flags:
-v, -- count Set log level, higher is more verbose
-h, --help help for iq
-a, --iq-application string Specify public application ID for request (required)
-x, --iq-server-url string Specify Nexus IQ Server URL (default "http://localhost:8070")
-x, --iq-server-url string Specify Nexus IQ Server URL (default "http://host.docker.internal:8070")
Copy link
Contributor

@bhamail bhamail Jan 19, 2021

Choose a reason for hiding this comment

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

Also curious about how this helps? (Faster, etc?)
I typically don't run IQ in a docker container, but I'm the last to claim my usage pattern is typical. ;)

Would the command ahab config (which allows you to set a local default value for the iq-server-url) be another solution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't run IQ in a docker container either - it is usually is on my team's host(laptop) - hence the default being the host.docker.internal - which says, my IQ is on my laptop, not in the container.

Base automatically changed from master to main February 25, 2021 18:50
@DarthHater DarthHater merged commit 86a2491 into main Jun 23, 2021
@DarthHater DarthHater deleted the tweaks-for-iq branch June 23, 2021 19:13
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.

3 participants