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

how is PartialServiceConfiguration class being used? #32

Open
qinfchen opened this issue Aug 16, 2017 · 10 comments
Open

how is PartialServiceConfiguration class being used? #32

qinfchen opened this issue Aug 16, 2017 · 10 comments

Comments

@qinfchen
Copy link
Contributor

qinfchen commented Aug 16, 2017

ClientConfiguration can be created using ServiceConfiguration and there is no easy way to convert PartialServiceConfiguration to ServiceConfiguration or vice versa even though they are pretty much identical except for the absence of security. I would like know more about how we intend to use the PartialServiceConfiguration class

EDIT: saw ServiceConfigurationFactor.propagateDefaults and answered my question above.

@qinfchen
Copy link
Contributor Author

qinfchen commented Aug 16, 2017

Can we make ServiceConfigurationFactory.propagateDefaults public?

@uschi2000
Copy link
Contributor

What's the use-case for this?

@qinfchen
Copy link
Contributor Author

Having a PartialServiceConfiguration declare outside the service discovery block like multipass'

auth:
  legacyModeEnabled: false
  multipass:
    security:
      trustStorePath: var/security/trustStore.jks
    uris:
    - https://multipass.palantir.dev:8643/multipass/api

or slate's postgate data source config

- dataSourcePoolCacheTimeout: 1m
  databaseName: palantir
  driver: org.postgresql.ds.PGSimpleDataSource
  maximumDataSourcePoolSize: 30
  name: Foundry
  portNumber: 5432
  postgate:
    uris: https://spp.palantir.dev:6473/postgate/api/
  serverName: postgres.palantir.dev
  type: foundry

It'd be great if we can specify service configuration outside of service discovery without requiring ssl block. Unless we are encouraging teams to only take service configuration from service discovery?

@uschi2000
Copy link
Contributor

In order to use propagateDefaults, you'd need a handle on a ServiceConfigBlock; if you have one of those, why not use ServiceConfigurationFactory#get ?

@qinfchen
Copy link
Contributor Author

qinfchen commented Aug 17, 2017

ServiceConfigBlock may not have the corresponding service config block for the PartialServiceConfiguration. I wonder if we should expose a convenient method of converting PartialServiceConfiguration to a ServiceConfiguration by filling in the defaults that are not present, which is what propagateDefaults does mostly

@uschi2000
Copy link
Contributor

Such a method exists: build a custom ServiceConfigBlock and then call get().

@meditativeape
Copy link
Contributor

ServiceConfigurationFactory#get only works for services defined in the ServicesConfigBlock. In some places, we define partial service configurations outside of the service discovery block, for example workers, and event consumers. The defaults in ServicesConfigBlock should get propagated to those as well, but currently there's no good way of doing that.

@ash211
Copy link
Contributor

ash211 commented Oct 3, 2017

I had to do this also, workaround is to build a ServicesConfigBlock with a named service and then retrieve the same service back.

    protected static final PartialServiceConfiguration STATS_CONFIG =
            PartialServiceConfiguration.of(Lists.newArrayList(STATS_URI + "api/"), Optional.of(SSL_CONFIGURATION));

<snip>

        // TODO(aash): is going through a ServiceConfigurationFactory the best way to convert
        // PartialServiceConfiguration -> ClientConfiguration ?
        stats = JaxRsClient.create(FoundryStatsService.class, USER_AGENT, ClientConfigurations.of(
                ServiceConfigurationFactory.of(
                        ServicesConfigBlock.builder().putServices("foundry-stats", STATS_CONFIG).build()).get(
                        "foundry-stats")));

@uschi2000 is this what you intend?

@uschi2000
Copy link
Contributor

We can add a static method ServiceConfigurationFactory that turns a partial config into a full one, or throws if not all required fields are set.

@qinfchen
Copy link
Contributor Author

qinfchen commented Oct 4, 2017

+1, created a pr for this https://github.com/palantir/http-remoting-api/pull/54

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

4 participants