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

feat(http.cr): default to HTTPS #70

Merged
merged 10 commits into from
Apr 15, 2020
Merged

Conversation

stakach
Copy link
Contributor

@stakach stakach commented Feb 18, 2020

Probably shouldn't be doing anything over HTTP anyway, so I would not consider this a breaking change.
With HTTP I was getting this error and it seems onerous to have to specify a @custom_endpoint for HTTPS

Requests specifying Server Side Encryption with AWS KMS managed keys must be made over a secure connection

Probably shouldn't be doing anything over HTTP anyway, so I would not consider this a breaking change.
With HTTP I was getting this error and it seems onerous to have to specify a `@custom_endpoint` for HTTPS

`Requests specifying Server Side Encryption with AWS KMS managed keys must be made over a secure connection`
stakach and others added 3 commits February 18, 2020 13:44
fixes compatibility with crystal 0.33
there is no queuing in the crystal HTTP library so if two requests occur on different fibers they both write to the same IO
This typically causes requests to fail as the S3 hash check fails
@stakach
Copy link
Contributor Author

stakach commented Apr 3, 2020

@taylorfinnell fixed up the spec and a couple of other issues i ran into when using this in production.
Would you consider reviewing this pull request?

@taylorfinnell
Copy link
Owner

Hey, thanks! Happy to merge this, looks like some tests are still failing and I think it needs a crystal tool format

@stakach
Copy link
Contributor Author

stakach commented Apr 9, 2020

Sweet, that was probably it.
crystal spec and ameba are clean on my machine

@stakach
Copy link
Contributor Author

stakach commented Apr 9, 2020

Ahh now it's a crystal 0.34 error.
I can quickly patch that

couldn't make crystal tool format find any issues on linux or macos - so not sure what travis is complaining about.
@stakach
Copy link
Contributor Author

stakach commented Apr 9, 2020

@taylorfinnell I literally cannot replicate the issue with crystal tool format and ./spec/awscr-s3/bucket_spec.cr that we're seeing on travis... all other specs pass

@taylorfinnell
Copy link
Owner

Yea, I can't repro it locally either, very weird. I will go ahead and merge this. Thank you!

@taylorfinnell taylorfinnell merged commit 252c6a1 into taylorfinnell:master Apr 15, 2020
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.

2 participants