Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Nov 06 15:54

    velo on encoded-documentation

    (compare)

  • Nov 06 15:54

    velo on master

    Deprecated `encoded` and add co… (compare)

  • Nov 05 20:57

    velo on encoded-documentation

    Deprecated `encoded` and add co… (compare)

  • Nov 05 20:00

    velo on encoded-documentation

    Deprecated `encoded` and add co… (compare)

  • Oct 30 21:34

    velo on master

    prepare release 10.6.0 [travis skip] updating versions… (compare)

  • Oct 30 21:32

    velo on 10.6.0

    prepare release 10.6.0 (compare)

  • Oct 30 21:26

    velo on master

    preparing for 10.6.0 release (compare)

  • Oct 30 21:23

    velo on master

    Add changes for 10.6 release (compare)

  • Oct 30 21:11

    velo on master

    [Proposal] generate mocked clie… (compare)

  • Oct 24 18:25

    velo on master

    Update README.md (#1101) Chang… (compare)

  • Oct 22 00:48

    velo on master

    Expose Method and Target on Req… (compare)

  • Oct 22 00:36

    velo on master

    Fixes a typo in Contract.java m… (compare)

  • Oct 22 00:36

    velo on master

    1048 simple illustration - more… (compare)

  • Oct 12 03:07

    velo on test-apt

    (compare)

  • Oct 12 03:06

    velo on test-apt

    First attempt at test stub code… Using templating framework Cleanup dependencies (compare)

  • Oct 12 03:05

    velo on github-tests

    (compare)

  • Oct 12 03:05

    velo on master

    Ignore github example integrati… (compare)

  • Oct 12 01:18

    velo on github-tests

    Ignore github example integrati… (compare)

  • Oct 04 20:22

    velo on master

    Add composed Spring annotations… (compare)

  • Oct 02 22:03

    velo on 10.5.1

    prepare release 10.5.1 (compare)

bitsofinfo
@bitsofinfo
I guess I don't follow
Adrian Cole
@adriancole
ex if you are flexible enough to try what I've mentioned several times, ex re-dispatch 409 to 200 in a ClientWrapper as opposed to changing the internal design and api design of feign
you can literally do what you want today
lemme show..
bitsofinfo
@bitsofinfo
Sure, well given there is no alternative that is my only option. You do understand though how that is a bit clunky
Adrian Cole
@adriancole
it is all about tradeoffs
your proposal would imply breaking the design (ex deleting ErrorDecoder) etc
that's a fairly severe response to the concern
in your world, it is normal for 409 to be treated as success.. but that's hardly good enough reason to break api
nor add even more fragile if statements and switches into the codebase
which are basically bug factories, as changes like this are high maintenance
bitsofinfo
@bitsofinfo
Just tossing out ideas, there are ways to approach implementation that support backwards compatibility. I'm not suggesting breaking functionality for people of course. Anyways, I'll shut up. Going to look at this client thing to mutate the response codes
Adrian Cole
@adriancole
in latest feign, there's a Response builder now
so we're talking about a 5 line wrapper to do what you want to do
    Feign.builder()
        .client((request, options) -> {
          Response response = delegate.execute(request, options);
          return (response.status() == 409)
              ? response.toBuilder().status(201).build()
              : response;
        })
Adrian Cole
@adriancole
where delegate is default (ex. Client delegate = new Client.Default(null, null);), or okhttp or whatever your type is
what your bumping into is basically one of the key things about feign's change culture..
adding hooks is a last resort.. we want people to ask for it, and in response we make sure there's a way to get by (ex ^^)
that helps extend the lifetime of the codebase at the cost of annoying people when they don't see a feature for each edge case called out as top-level
now we probably do need a cookbook of how to do edge cases, like remapping exceptions etc
(tl;dr; you aren't crazy and your response to not having a top-level feature for X isn't unusual)
bitsofinfo
@bitsofinfo
I get it. Thanks for the pointer here with this wrapper. I'm going to look into it to see if it can work for my cases.
Adrian Cole
@adriancole
:thumbsup:
bitsofinfo
@bitsofinfo
so like this
        @Bean
    @Scope("prototype")
    @Primary
    @ConditionalOnMissingBean
    @ConditionalOnProperty(name = "feign.hystrix.enabled", matchIfMissing = true)
    public Feign.Builder feignHystrixBuilder() {

        return HystrixFeign.builder().client(new Client() {

            private Client client = new Client.Default(null, null);

            @Override
            public Response execute(Request request, Options options) throws IOException {
                Response resp = client.execute(request, options);

                /*
                 * Impl all the custom mutations I want here
                 *  right here
                 */

                return resp;
            }

        });
    }
Adrian Cole
@adriancole
ya unless you want to inject the client
making that a separate bean
bitsofinfo
@bitsofinfo
yep
Adrian Cole
@adriancole
probably don't need ConditionalOnMissingBean since you are intentionally defining it
bitsofinfo
@bitsofinfo
yeah just took that out, trying to figure out why its not being invoked
Adrian Cole
@adriancole
spring-cloud-netflix I presume?
if so..
@Configuration
public class FooConfiguration {
    @Bean
    @Scope("prototype")
    public Feign.Builder feignBuilder() {
        return Feign.builder();
    }
}
bitsofinfo
@bitsofinfo
yes, thats what I have, still working on it
bitsofinfo
@bitsofinfo
hmm, bean only gets created if I rename the beanName(), but this little custom Client wrapper never gets invoked on requests. Must be some precendence thing in the context load
Adrian Cole
@adriancole
maybe try here https://gitter.im/spring-cloud/spring-cloud I don't know off top of head
bitsofinfo
@bitsofinfo
yeah, I'll figure it out. Thanks for the help, once this works it should solve a lot of my needs
Adrian Cole
@adriancole
good luck!
bitsofinfo
@bitsofinfo
I think its the TraceFeignClient is overwriting the one I set
Adrian Cole
@adriancole
oh the one in spring cloud sleuth
looks like you might be able to define as a bean..
bitsofinfo
@bitsofinfo
yeah prob define as separate bean and then it will be picked up
bitsofinfo
@bitsofinfo
that works, now hostresolution fails cause the one I actually need to proxy is the LoadBalancerFeignClient
Adrian Cole
@adriancole
if it becomes impossible to wire ping here https://gitter.im/spring-cloud/spring-cloud-sleuth
bitsofinfo
@bitsofinfo
yeah I need to move the convo over there
Rocky
@vineyverma
@vineyverma
Hey Everyone, is it not okay to use GSON marshalling/unmarshalling with Feign
I ran into some problems when due to the way some APIs which I were calling could not be marshalled so I got them as string and used gson to marshall it
is that wrong practise?
Adrian Cole
@adriancole
if so, you can provide a more configured gson as a ctor arg to GsonDecoder
Adrian Cole
@adriancole
feign 9.3 on the way with fallbackfactory