nobodyiam commented on a change in pull request #2160: [Dubbo-2030] A basically running demo implementation for introducing dynamic configuration to Dubbo. URL: https://github.com/apache/incubator-dubbo/pull/2160#discussion_r210879731
########## File path: dubbo-config/dubbo-config-dynamic/src/main/java/org/apache/dubbo/config/dynamic/support/apollo/ApolloDynamicConfiguration.java ########## @@ -0,0 +1,134 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.dubbo.config.dynamic.support.apollo; + +import com.ctrip.framework.apollo.Config; +import com.ctrip.framework.apollo.ConfigChangeListener; +import com.ctrip.framework.apollo.ConfigService; +import com.ctrip.framework.apollo.enums.PropertyChangeType; +import com.ctrip.framework.apollo.model.ConfigChange; +import com.ctrip.framework.apollo.model.ConfigChangeEvent; +import org.apache.dubbo.common.Constants; +import org.apache.dubbo.common.URL; +import org.apache.dubbo.config.dynamic.AbstractDynamicConfiguration; +import org.apache.dubbo.config.dynamic.ConfigChangeType; +import org.apache.dubbo.config.dynamic.ConfigType; +import org.apache.dubbo.config.dynamic.ConfigurationListener; + +/** + * + */ +public class ApolloDynamicConfiguration extends AbstractDynamicConfiguration { + private static final String APOLLO_ENV_KEY = "env"; + // FIXME the key? + private static final String APOLLO_ADDR_KEY = ""; + private static final String APOLLO_CLUSTER_KEY = "apollo.cluster"; + /** + * support two namespaces: application -> dubbo + */ + private Config dubboConfig; + private Config appConfig; + + public ApolloDynamicConfiguration() { + + } + + @Override + public void init() { + /** + * Instead of using Dubbo's configuration, I would suggest use the original configuration method Apollo provides. + */ +// String configEnv = env.getCompositeConf().getString(ENV_KEY); +// String configCluster = env.getCompositeConf().getString(CLUSTER_KEY); + String configEnv = url.getParameter(Constants.CONFIG_ENV_KEY); + String configAddr = url.getParameter(Constants.CONFIG_ADDRESS_KEY); + String configCluster = url.getParameter(Constants.CONFIG_CLUSTER_KEY); + if (configEnv != null) { + System.setProperty(APOLLO_ENV_KEY, configEnv); + } + if (configAddr != null) { + System.setProperty(APOLLO_ADDR_KEY, configAddr); + } + if (configCluster != null) { + System.setProperty(APOLLO_CLUSTER_KEY, configCluster); + } + + dubboConfig = ConfigService.getConfig("dubbo"); Review comment: @chickenlj Yes, `appConfig = ConfigService.getAppConfig();` will only read the `application` namespace of the current app.id. For now, the public namespace is always prefixed with department id to ensure its uniqueness. As the screenshot below, the `TEST2` is the department id, and the public namespace name will be `TEST2.dubbo`  You may have a try with this [link](http://140.143.100.23:8070/namespace.html?#/appid=dubbo-test)，user name: `admin`, password: `apollo`。 Private namespace is not necessary to have department id as its prefix, so that it could be named as `dubbo`。 In my opinion, users may configure dubbo related configurations in its private namespace like `application`, `dubbo`, `rpc`, etc for small sized companies, or in some public namespace like `common.dubbo`, `base.dubbo` for medium to large companies. What I mean here is those meduim to large companies is more likely to have a rpc framework team who will maintain an optimized global default configuration. In some cases, I think users may even configure dubbo related configurations in more than one namespace. For dubbo, I think we don't actually care what the namespace name is or how many namespaces are there, what we actually care is the configurations in it. So I would suggest we let the user pass the namespace name as an optional parameter. If it's not provided, we could fallback to `application` namespace (may be better) or `dubbo` namespace. If it's provided, then we could just use it. We could even support a comma separated namespace name parameter such as `application,dubbo` if that's necessary. With that said, I think the namespace name should not be hard-coded as `application` or `dubbo`, they are very good fallback namespace names, but it's not good to force the user to use these name. Simply because we don't actually care the names, and users may have a hundred reasons to have a different name. BTW, I would be happy to submit a pull request to `chickenlj:dynamicconfig` so that my ideas could be expressed more clearly. So just let me know if it's necessary :-) ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
