feat(config): add support for specifying port#7
Merged
Conversation
rwjblue
force-pushed
the
rwjblue/push-srxtklwwmwqr
branch
from
9c639b0 to
9cb9856
Compare
rwjblue
force-pushed
the
rwjblue/push-srxtklwwmwqr
branch
from
9cb9856 to
794557c
Compare
rwjblue
changed the title
rwjblue
force-pushed
the
rwjblue/push-srxtklwwmwqr
branch
2 times, most recently
from
1661a39 to
f849191
Compare
rwjblue
commented
Apr 4, 2025
Comment on lines
-57
to
-64
| if !strings.HasSuffix(host, ".glean.com") { | ||
| return "", fmt.Errorf("invalid host format. Must be either 'instance' or 'instance-be.glean.com'") | ||
| } | ||
|
|
||
| if !strings.HasSuffix(strings.TrimSuffix(host, ".glean.com"), "-be") { | ||
| return "", fmt.Errorf("invalid host format. Must end with '-be.glean.com'") | ||
| } | ||
|
|
Collaborator
Author
There was a problem hiding this comment.
Removing this allows usage of custom domains or IP's which is important to support the proxy use case.
rwjblue
commented
Apr 4, 2025
Comment on lines
-133
to
-144
| { | ||
| name: "invalid domain", | ||
| input: "linkedin.example.com", | ||
| wantErr: true, | ||
| errContains: "invalid host format", | ||
| }, | ||
| { | ||
| name: "missing -be suffix", | ||
| input: "linkedin.glean.com", | ||
| wantErr: true, | ||
| errContains: "invalid host format", | ||
| }, |
Collaborator
Author
There was a problem hiding this comment.
These tests are removed because we actually do support this case now
Often times you might need to configure a custom host or IP (e.g. for a proxy) and you might then need to specify a port for that connection to function. This adds support for that. **Note:** This removes the validations of various hosts (e.g. you can specify any host, including an ip address), but it still supports using `--host foo` and having that expand to `foo-be.glean.com`. - Add port field to ConfigOptions struct - Update CLI flags and help text to include port option - Modify HTTP client to use port in base URL construction - Add support for foo.bar.com host format - Add tests for port configuration and URL building
rwjblue
force-pushed
the
rwjblue/push-srxtklwwmwqr
branch
from
f849191 to
9f83ae9
Compare
scalvert
approved these changes
Apr 6, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Often times you might need to configure a custom host or IP (e.g. for a proxy) and you might then need to specify a port for that connection to function. This adds support for that.
Note: This removes the validations of various hosts (e.g. you can specify any host, including an ip address), but it still supports using
--host fooand having that expand tofoo-be.glean.com.