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

Allow passing headers per request #268

Closed
grosser opened this issue Sep 13, 2017 · 7 comments
Closed

Allow passing headers per request #268

grosser opened this issue Sep 13, 2017 · 7 comments

Comments

@grosser
Copy link
Contributor

grosser commented Sep 13, 2017

Atm I'm trying to update statefulset so I need to do this hack-around which is a bit ugly ...

      def deploy
        ....
        with_patch_header do
          client.patch_statefulset name, namespace, [{op: "replace", path: "/spec", value: @template.fetch(:spec)}]
        end
      end

      private
      
      def with_patch_header
        old = client.headers['Content-Type']
        client.headers['Content-Type'] = 'application/json-patch+json'
        yield
      ensure
        client.headers['Content-Type'] = old
      end
@moolitayer
Copy link
Collaborator

We have patch entity, would that serve your use case @grosser ?

    def patch_entity(resource_name, name, patch, namespace = nil)
      ns_prefix = build_namespace_prefix(namespace)
      handle_exception do
        rest_client[ns_prefix + resource_name + "/#{name}"]
          .patch(
            patch.to_json,
            { 'Content-Type' => 'application/strategic-merge-patch+json' }.merge(@headers)
          )
      end
    end

@grosser
Copy link
Contributor Author

grosser commented Sep 14, 2017

I need to change the Content-Type ... I'm using patch_entity already

@moolitayer
Copy link
Collaborator

Oh right, so it's 'Content-Type' => 'application/strategic-merge-patch+json' vs 'application/json-patch+json'? Can you explain what is the difference? I think it was discussed in #161 but don't remember the distinction.

@grosser
Copy link
Contributor Author

grosser commented Sep 14, 2017

idk ... was following an example and the default one did not work ... have not looked any deeper :D

@cben
Copy link
Collaborator

cben commented Mar 4, 2018

doc: https://kubernetes.io/docs/tasks/run-application/update-api-object-kubectl-patch/

Are there other use cases than patch to allow direct setting of headers?
My instinct is we're clearly missing functionality in patch_*() and should add it, and defer the general escape hatch until proven necessary.

Current signature is patch_foo(name, patch, namespace = nil). It's awkward that we have name & namespace not adjacent :-(, and I'd like to avoid adding a 4th positional arg to it. Cf. #312.

@stormbeta
Copy link

@cben That awkwardness with the method signature is why I didn't try to add it in a PR myself - wasn't sure what the best way to handle it would be.

It's also worth noting that some CustomResourceDefinitions may require this if they don't support strategic merge.

@cben
Copy link
Collaborator

cben commented Jan 23, 2019

Patch formats are covered by #357, more discussion there. Seems we're all reluctant about overloading existing method, so best direction is adding new methods eg. json_patch_* and merge_patch_*. I'm not working on it, PRs welcome.

Closing as I'm not aware of other use cases for per-request headers, but I'm opening a new issue for HTTP requests to arbitrary paths... => #389

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants