Make opts.hostedPath (and installation) respect the s3ForcePathStyle param
#608
Conversation
| } | ||
| // If we are using s3ForcePathStyle the bucket is part of the http object path | ||
| // (but not the S3 key prefix path). If we aren't this isn't as much of a consideration. | ||
| const bucket_path = config.s3ForcePathStyle ? `/${config.bucket}/` : '/'; |
There was a problem hiding this comment.
Moving this below the if/else structure does skip an early return on line 21. I think that early return is acting in light of an error condition so not having config.prefix set for it is fine, but I'm not 100% sure.
|
Hi @jared-duo - thank you for this contribution. Looks totally reasonable to me. Have you confirmed/been able to test that it fixes the original issue? If so, can you think of a way to add a test to the unit tests to ensure this does not regress ever in the future? |
|
I have verified that this works for my use case. (Using the forked repo currently.) I'll TAL at the unit tests as soon as I can and see if I can come up with a reasonable way to test this - hopefully in the next week or so. Thanks for the feedback! |
|
#650 fixed #607. @jared-duo can you test and see if this PR is still needed or should we close it? |
@benmccann ou can close it. |
This is an example of a potential fix for issue #607.
I'm sure I may be doing something non-idiomatically as I do not code for NodeJS too often. Also guessing that the CHANGELOG and package version will need incremented appropriately if this is actually something that
node-pre-gypwould like to do (and I'm not missing something.)