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

Feature/support https #6

Merged
merged 19 commits into from
Feb 28, 2023
Merged

Conversation

amanica
Copy link
Contributor

@amanica amanica commented Feb 27, 2023

hi,
I'd like to use this library to connect to influxdb cloud, but for that I need https support, so I gave that a go.
In order to be backwards compatible but still allow configuring an https url, I suggest we move away from INFLUXDB_HOST to INFLUXDB_URL

I also fixed some bugs I ran into while testing this

  • support influx org names that contain a space, by converting it to "%20"
  • in query.jl line 57 I filter out lines where _time=_time apparently influx is happy to repeat headers:
    Selection_528

Let me know if you need me to change anything.

@amanica
Copy link
Contributor Author

amanica commented Feb 27, 2023

I don't think this github ci pipeline is configured correctly, hence the pipeline is failing: https://stackoverflow.com/a/67998780/381083
with my changes, it should be possible to test agains a influx cloud db

@kafisatz
Copy link
Owner

I don't think this github ci pipeline is configured correctly, hence the pipeline is failing: https://stackoverflow.com/a/67998780/381083 with my changes, it should be possible to test agains a influx cloud db

I am not sure why the CI is not working. Clearly it used to work a month ago: https://github.com/kafisatz/InfluxDBClient.jl/actions/runs/3845052229/jobs/6548750295

@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2023

Codecov Report

Merging #6 (cb52794) into main (378269c) will decrease coverage by 86.45%.
The diff coverage is 39.28%.

❗ Current head cb52794 differs from pull request most recent head 6769f87. Consider uploading reports for the commit 6769f87 to get more accurate results

@@            Coverage Diff             @@
##             main      #6       +/-   ##
==========================================
- Coverage   94.02%   7.58%   -86.45%     
==========================================
  Files           8       8               
  Lines         335     343        +8     
==========================================
- Hits          315      26      -289     
- Misses         20     317      +297     
Impacted Files Coverage Δ
src/delete.jl 0.00% <0.00%> (-96.30%) ⬇️
src/query.jl 0.00% <0.00%> (-92.23%) ⬇️
src/write.jl 0.00% <0.00%> (-97.44%) ⬇️
src/buckets.jl 5.17% <22.22%> (-87.94%) ⬇️
src/settings.jl 48.88% <81.81%> (-43.22%) ⬇️
src/lineprotocol.jl 0.00% <0.00%> (-95.32%) ⬇️
src/metadata.jl 0.00% <0.00%> (-94.45%) ⬇️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kafisatz
Copy link
Owner

I don't think this github ci pipeline is configured correctly, hence the pipeline is failing: https://stackoverflow.com/a/67998780/381083 with my changes, it should be possible to test agains a influx cloud db

I am not sure why the CI is not working. Clearly it used to work a month ago: https://github.com/kafisatz/InfluxDBClient.jl/actions/runs/3845052229/jobs/6548750295

@amanica I fixed the CI. It seems that it is working with InfluxDB 2.4, but 'latest' (2.6?) does not seem to work. Might be about default settings or http vs https. Notably I have not tested https (neither locally nor in CI). Also I can't test influxdb cloud.

@kafisatz kafisatz merged commit 8b3fe59 into kafisatz:main Feb 28, 2023
@amanica
Copy link
Contributor Author

amanica commented Feb 28, 2023

thanks for sorting out and merging!

@amanica amanica deleted the feature/support_https branch February 28, 2023 14:40
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.

None yet

3 participants