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

Constructor overload that includes swarmMgrUri #22

Open
anton-johansson opened this issue Aug 8, 2018 · 4 comments
Open

Constructor overload that includes swarmMgrUri #22

anton-johansson opened this issue Aug 8, 2018 · 4 comments

Comments

@anton-johansson
Copy link

I find it strange that all properties can be set through a constructor of SwarmMemberAddressProvider except swarmMgrUri (and skipVerifySsl, but that is of less importance). It can only be set through an environment variable. I would like to keep my settings in one place, and I'm currently creating my Hazelcast config programmatically, like this:

        NetworkConfig network = new NetworkConfig();
        network.setPublicAddress(publicAddress);
        network.setPort(port);
        network.setPortAutoIncrement(false);
        network.setJoin(join);

        if (dockerSwarmEnabled)
        {
            SwarmMemberAddressProvider memberAddressProvider = new SwarmMemberAddressProvider(dockerSwarmNetworkName, "", dockerSwarmServiceName, port);

            MemberAddressProviderConfig memberAddressProviderConfig = new MemberAddressProviderConfig();
            memberAddressProviderConfig.setEnabled(true);
            memberAddressProviderConfig.setImplementation(memberAddressProvider);

            network.setMemberAddressProviderConfig(memberAddressProviderConfig);
        }

... and there's no way of setting the swarm manager URI. :(

@anton-johansson
Copy link
Author

anton-johansson commented Aug 8, 2018

Actually, it might make more sense to set these specific settings through environment variables... Considering they aren't really application settings, but rather settings related to the Swarm cluster.

I'll think about this for a while, then maybe close the issue.

@bitsofinfo
Copy link
Owner

having it set via constructor option is not bad either, as long as still works as-is, please submit a PR and will put it in, won't hurt

@anton-johansson
Copy link
Author

anton-johansson commented Aug 8, 2018

Yeah, if I need the constructor, I'll gladly submit a PR!

However, as I was thinking about setting them all through environment variables, I noticed that only the swarm manager URI can be set through environment variables. So that plans goes out the window as well. :(

What would you think about allowing setting them all through environment variables? For example, first checking the system properties. If they aren't set, check environment variables. If they aren't set, it will obviously fail.

EDIT: Actually, constructor is good. I can handle the environment variables myself!

@bitsofinfo
Copy link
Owner

Sure, please submit a PR to make all the configurable probs editable via env vars. Thanks!

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

2 participants