-
Notifications
You must be signed in to change notification settings - Fork 24
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
feat: init api server configs with path of host #511
base: main
Are you sure you want to change the base?
Conversation
Hi @jeffreyssmith2nd i saw you created the last release. Maybe you can take a look at this PR? Thank you :) |
Hi @srebhan can you support us here? I think it is less effort then the last one ;-) If not, whom can we contact here? |
@albrecht-j sorry but this is not my field of expertise. ;-) |
Hey @albrecht-j, I'm the right person for this. My apologies, I just lost track of this PR. I will get it reviewed before the next release. |
Hey @jeffreyssmith2nd |
Hi @jeffreyssmith2nd, |
do you have a schedule for the next release? |
Seems a function duplicatie to #530. But the InfluxDB team doesn't care. I can have my fork but perhaps switch to some SDK. I remember Python SDK v1 has such path. Not sure if v2 has. |
// initialize api server configurations with path of host url | ||
apiConfig.Servers = ServerConfigurations{{URL: params.Host.Path}} | ||
for key := range apiConfig.OperationServers { | ||
apiConfig.OperationServers[key] = ServerConfigurations{{URL: params.Host.Path}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this looks relatively straightforward and like it will address the issue.
My one reservation is that it completely overrides the existing apiConfig.OperationServers[key]
value. We don't lose any valuable information today (just a non-useful description), but I'm concerned this could introduce subtle issues in the future if more valuable information is already populated in the ServerConfigurations
.
Would it be possible to set the URL
value without overwriting everything else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, what is the current state of this PR? Thanks. |
Reason:
If the influxdb server runs behind a reverse proxy with url e.g. "https://anyhost/influxdb" the influx-cli could not access the url because the host base path of the api url is always empty.
Idea:
The information of the base can be passed in the cli with the host parameter like this:
influx backup --host "https://anyhost/influxdb"
Implementation:
The base path is available in the parameter
params.Host.Path
and could be forwarded to the OperationServers of the generated client API