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

Req.merge: Deep merge options #319

Open
wojtekmach opened this issue Mar 8, 2024 · 2 comments
Open

Req.merge: Deep merge options #319

wojtekmach opened this issue Mar 8, 2024 · 2 comments

Comments

@wojtekmach
Copy link
Owner

wojtekmach commented Mar 8, 2024

Right now we special case :headers and :params. For example:

Req.new(headers: [a: 1]) |> Req.merge(headers: [b: 2]) == Req.new(headers: [a: 1, b: 2])

Another one when this might be useful is aws_sigv4:

Req.new(aws_sigv4: ...) |> Req.merge(aws_sigv4: [service: :s3])

We could special case that one too or have a more generic mechanism when registering the options, something like:

Req.Request.register_options(req, aws_sigv4: [update: :replace | :merge])
@wojtekmach
Copy link
Owner Author

Req.Request.register_options(req, aws_sigv4: [update: :replace | :merge])

this choice would prevent us from validating nested option names like:

Req.Request.register_options(req, aws_sigv4: [:access_key_id, :region, ...])

so need to carefully consider it, maybe we can have it both ways but maybe we don't.

@gaberduty
Copy link

I found myself on this issue when looking into merging the json option. I'm using Req right now to query DynamoDB, and it's got a pretty verbose request form:

Req.post(
  req,
  headers: %{"X-Amz-Target" => "DynamoDB_20120810.Query"},
  json: %{
    "TableName" => "tablename",
    "IndexName" => "indexname",
    "KeyConditionExpression" => "someVal = :someval",
    "ExpressionAttributeValues" => %{
      ":someval" => %{
        "S" => "the value you're querying"
      }
    }
  }
)

If the response includes a %{"LastEvaluatedKey" => key}, then you have to make a followup request that repeats all of that, but also includes the key:

Req.post(
  req,
  headers: %{"X-Amz-Target" => "DynamoDB_20120810.Query"},
  json: %{
    "TableName" => "tablename",
    "IndexName" => "indexname",
    "KeyConditionExpression" => "someVal = :someval",
    "ExpressionAttributeValues" => %{
      ":someval" => %{
        "S" => "the value you're querying"
      }
    },
    "ExclusiveStartKey" => key
  }
)

It would be nice to just set the base json in Req.new and then merge in that key if necessary. As it is, I'm doing

req = update_in(req.options.json, fn j -> Map.put(j, "ExclusiveStartKey", key) end)

but I'm not sure if that's kosher. I'm wary of messing directly with structs beneath the exposed functional interface.

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

No branches or pull requests

2 participants