Allow options like "timeout" to be passed to HTTPoison.get_blob underneath inside options
#54
Conversation
This file contains 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
…word list Co-authored-by: Hitalo Siqueira <[email protected]> Co-authored-by: Jon Lauridsen <[email protected]>
…son from Azurex for `get_blob` Co-authored-by: Jon Lauridsen <[email protected]>
Co-authored-by: Jon Lauridsen <[email protected]>
Allow passing connection parameters together with the container to blob functions. Values from the parameters are prioritized over default configuration.
Still always read azure connection string from application config, but allow the element itself to be parameterised. Still default to azurex. This change does introduce some breaking API changes, notably on put_blob, to nest params under options (similar to an earlier commit's work on get_blob). Co-authored-by: Aman Kumar Singh <[email protected]> Co-authored-by: Konstantinos Gkinis <[email protected]>
|
Was it necessary to add the config element changes in the same PR? At first glance they look like separate goals. I'm not a maintainer, just asking the question. |
|
@mattpolzin sorry, these are definitely separate goals. When I recently committed to this fork's master branch the other day, I had no idea this PR existed. @krishna1m let's close this, cut a branch before that recent fork commit, and PR that. If that lands, we can PR the config element bit separately. WDYT? |
This reverts commit c8f56b2.
Co-authored-by: Aman Kumar Singh <[email protected]>
Pave the way for merging in a branch from another fork that has not been accepted upstream yet, which adds support for multiple azure storage accounts, parameterised by call. Previous commits in our fork here made breaking changes to a few functions, e.g. `put_blob()`. Upstream, this function's final argument is a keyword list defaulting to `[]`. In our fork, we've nested the params keyword list inside another one, so that we can pass various non-param overrides without having 2 keyword lists. The breaking change isn't obvious in the type spec, but the user must pass a differently-shaped keyword list. The noaccOS fork has a similar motivation to add non-params overrides, but they've taken the other approach: passing 2 keyword lists. When we merge in their branch here, we'll have to pick a signature kind (2 keyword lists or a single nested one). Since we're already using a nested one, we'll stick with that pattern and resolve the merge conflicts. First though, we must finish the interface change job that we'd started, and nest params inside options for all functions. Interestingly, for some functions, this is just a rename to make it clear we'd already implicitly done this. Co-authored-by: Harisudhan V R <[email protected]>
…into krishna1m-master
And also fix all tests. Co-authored-by: Aman Kumar Singh <[email protected]> Co-authored-by: Harisudhan V R <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #53
Allows to call the new signature like below -