[GitHub] cloudstack-docs-admin pull request: Update provisioning.rst
GitHub user sedukull opened a pull request: https://github.com/apache/cloudstack-docs-admin/pull/1 Update provisioning.rst Edited a link You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/cloudstack-docs-admin patch-1 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack-docs-admin/pull/1.patch commit b0d7b4d3543aacac934eb56cae803a4dca8a3d50 Author: sedukull Date: 2014-02-13T12:02:08Z Update provisioning.rst
[GitHub] cloudstack-docs-rn pull request: Update rnotes.rst
GitHub user sedukull opened a pull request: https://github.com/apache/cloudstack-docs-rn/pull/1 Update rnotes.rst Added few changes to alignments. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/cloudstack-docs-rn patch-1 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack-docs-rn/pull/1.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1 commit 4f7df21c779e51183e7ca675af5785b267362f2b Author: sedukull Date: 2014-02-13T16:19:58Z Update rnotes.rst Added few changes to alignments.
[GitHub] cloudstack-docs-install pull request: Update optional_installation...
GitHub user sedukull opened a pull request: https://github.com/apache/cloudstack-docs-install/pull/3 Update optional_installation.rst Fixed Few alignment issues You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/cloudstack-docs-install patch-1 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack-docs-install/pull/3.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3 commit 25b643439b6f222728779f50c498e478257b6cd9 Author: sedukull Date: 2014-02-13T17:23:10Z Update optional_installation.rst Fixed Few alignment issues
[GitHub] cloudstack-docs-install pull request: Update optional_installation...
Github user sedukull commented on the pull request: https://github.com/apache/cloudstack-docs-install/pull/3#issuecomment-35002947 Fixed Few alignment issues.
[GitHub] cloudstack-docs-install pull request: Update optional_installation...
Github user sedukull closed the pull request at: https://github.com/apache/cloudstack-docs-install/pull/3
[GitHub] cloudstack-docs-install pull request: Update optional_installation...
Github user sedukull commented on the pull request: https://github.com/apache/cloudstack-docs-install/pull/3#issuecomment-35003054 Not yet applied
[GitHub] cloudstack-docs-install pull request: Update optional_installation...
GitHub user sedukull reopened a pull request: https://github.com/apache/cloudstack-docs-install/pull/3 Update optional_installation.rst Fixed Few alignment issues You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/cloudstack-docs-install patch-1 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack-docs-install/pull/3.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3 commit 25b643439b6f222728779f50c498e478257b6cd9 Author: sedukull Date: 2014-02-13T17:23:10Z Update optional_installation.rst Fixed Few alignment issues
[GitHub] cloudstack-docs-rn pull request: Update rnotes.rst
Github user sedukull closed the pull request at: https://github.com/apache/cloudstack-docs-rn/pull/1
[GitHub] cloudstack-docs-install pull request: Update hypervisor_installati...
GitHub user sedukull opened a pull request: https://github.com/apache/cloudstack-docs-install/pull/4 Update hypervisor_installation.rst Fixed few alignment issues You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/cloudstack-docs-install patch-2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack-docs-install/pull/4.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #4 commit e7c99899edb61154f821771140d16acb5e43757c Author: sedukull Date: 2014-02-13T18:07:41Z Update hypervisor_installation.rst Fixed few alignment issues
[GitHub] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35534192
--- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
@@ -0,0 +1,473 @@
+// 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.cloudstack.report;
+
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.net.URL;
+import java.net.SocketTimeoutException;
+import java.net.MalformedURLException;
+import java.net.ProtocolException;
+import java.net.UnknownHostException;
+import java.io.OutputStreamWriter;
+import java.io.IOException;
+
+import javax.inject.Inject;
+import javax.net.ssl.HttpsURLConnection;
+
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+
+import org.apache.commons.codec.digest.DigestUtils;
+
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.dc.ClusterVO;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.component.ComponentMethodInterceptable;
+import com.cloud.utils.concurrency.NamedThreadFactory;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.upgrade.dao.VersionDao;
+import com.cloud.upgrade.dao.VersionVO;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.DiskOfferingVO;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.common.util.concurrent.AtomicLongMap;
+
+@Component
+public class UsageReporter extends ManagerBase implements
ComponentMethodInterceptable {
+public static final Logger s_logger =
Logger.getLogger(UsageReporter.class.getName());
+
+/* !FIX ME! This should point to a Apache Infra host with SSL! */
+private String reportHost = "https://call-home.cloudstack.org/report";;
+
+private String uniqueID = null;
+
+private static UsageReporter s_instance = null;
+
+private ScheduledExecutorService _executor = null;
+
+@Inject
+private ConfigurationDao _configDao;
+@Inject
+private HostDao _hostDao;
+@Inject
+private ClusterDao _clusterDao;
+@Inject
+private PrimaryDataStoreDao _storagePoolDao;
+@Inject
+private DataCenterDao _dataCenterDao;
+@Inject
+private UserVmDao _userVmDao;
+@Inject
+private VMInstanceDao _vmInstance;
+@Inject
+private VersionDao _versionDao;
+@Inject
+private DiskOfferingDao _diskOfferingDao;
+
+int usageReportInterval = -1;
+
+public static UsageReporter getInstance() {
+return s_instance;
+}
+
+public static UsageReporter getInstance(Map configs) {
+s_instance.init(configs);
+return s_instance;
+}
+
[GitHub] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35534267
--- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
@@ -0,0 +1,473 @@
+// 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.cloudstack.report;
+
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.net.URL;
+import java.net.SocketTimeoutException;
+import java.net.MalformedURLException;
+import java.net.ProtocolException;
+import java.net.UnknownHostException;
+import java.io.OutputStreamWriter;
+import java.io.IOException;
+
+import javax.inject.Inject;
+import javax.net.ssl.HttpsURLConnection;
+
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+
+import org.apache.commons.codec.digest.DigestUtils;
+
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.dc.ClusterVO;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.component.ComponentMethodInterceptable;
+import com.cloud.utils.concurrency.NamedThreadFactory;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.upgrade.dao.VersionDao;
+import com.cloud.upgrade.dao.VersionVO;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.DiskOfferingVO;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.common.util.concurrent.AtomicLongMap;
+
+@Component
+public class UsageReporter extends ManagerBase implements
ComponentMethodInterceptable {
+public static final Logger s_logger =
Logger.getLogger(UsageReporter.class.getName());
+
+/* !FIX ME! This should point to a Apache Infra host with SSL! */
+private String reportHost = "https://call-home.cloudstack.org/report";;
+
+private String uniqueID = null;
+
+private static UsageReporter s_instance = null;
+
+private ScheduledExecutorService _executor = null;
+
+@Inject
+private ConfigurationDao _configDao;
+@Inject
+private HostDao _hostDao;
+@Inject
+private ClusterDao _clusterDao;
+@Inject
+private PrimaryDataStoreDao _storagePoolDao;
+@Inject
+private DataCenterDao _dataCenterDao;
+@Inject
+private UserVmDao _userVmDao;
+@Inject
+private VMInstanceDao _vmInstance;
+@Inject
+private VersionDao _versionDao;
+@Inject
+private DiskOfferingDao _diskOfferingDao;
+
+int usageReportInterval = -1;
+
+public static UsageReporter getInstance() {
+return s_instance;
+}
+
+public static UsageReporter getInstance(Map configs) {
+s_instance.init(configs);
+return s_instance;
+}
+
[GitHub] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35605705
--- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
@@ -0,0 +1,487 @@
+// 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.cloudstack.report;
+
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.net.URL;
+import java.net.SocketTimeoutException;
+import java.net.MalformedURLException;
+import java.net.ProtocolException;
+import java.net.UnknownHostException;
+import java.io.OutputStreamWriter;
+import java.io.IOException;
+
+import javax.inject.Inject;
+import javax.net.ssl.HttpsURLConnection;
+
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+
+import org.apache.commons.codec.digest.DigestUtils;
+
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.dc.ClusterVO;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.component.ComponentMethodInterceptable;
+import com.cloud.utils.concurrency.NamedThreadFactory;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.upgrade.dao.VersionDao;
+import com.cloud.upgrade.dao.VersionVO;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.DiskOfferingVO;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.common.util.concurrent.AtomicLongMap;
+
+@Component
+public class UsageReporter extends ManagerBase implements
ComponentMethodInterceptable {
+public static final Logger s_logger =
Logger.getLogger(UsageReporter.class.getName());
+
+/* !FIX ME! This should point to a Apache Infra host with SSL! */
+private String reportHost = "https://call-home.cloudstack.org/report";;
+
+private String uniqueID = null;
+
+private static UsageReporter s_instance = null;
+
+private ScheduledExecutorService _executor = null;
+
+@Inject
+private ConfigurationDao _configDao;
+@Inject
+private HostDao _hostDao;
+@Inject
+private ClusterDao _clusterDao;
+@Inject
+private PrimaryDataStoreDao _storagePoolDao;
+@Inject
+private DataCenterDao _dataCenterDao;
+@Inject
+private UserVmDao _userVmDao;
+@Inject
+private VMInstanceDao _vmInstance;
+@Inject
+private VersionDao _versionDao;
+@Inject
+private DiskOfferingDao _diskOfferingDao;
+
+int usageReportInterval = -1;
+
+public static UsageReporter getInstance() {
--- End diff --
It seems we are using a singleton pattern here, but it seems not thread
safe?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
[GitHub] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35605790
--- Diff: reporter/usage-report-collector.py ---
@@ -0,0 +1,64 @@
+#!/usr/bin/env python
+# 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.
+
+from flask import abort, Flask, request, Response
+from elasticsearch import Elasticsearch
+import json
+import time
+
+def json_response(response):
+return json.dumps(response, indent=2) + "\n", 200, {'Content-Type':
'application/json; charset=utf-8'}
+
+def generate_app(config=None):
+app = Flask(__name__)
+
+@app.route('/report/', methods=['POST'])
+def report(unique_id):
+# We expect JSON data, so if the Content-Type doesn't match JSON
data we throw an error
+if 'Content-Type' in request.headers:
+if request.headers['Content-Type'] != 'application/json':
+abort(417, "No or incorrect Content-Type header was
supplied")
+
+index = "cloudstack-%s" % time.strftime("%Y.%m.%d", time.gmtime())
+timestamp = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime())
+
+es = Elasticsearch()
+es.indices.create(index=index, ignore=400)
+
+report = json.loads(request.data)
+report["unique_id"] = unique_id
+report["timestamp"] = timestamp
+
+es.index(index=index, doc_type="usage-report",
body=json.dumps(report), timestamp=timestamp, refresh=True)
+
+response = {}
+return json_response(response)
+
+return app
+
+
+app = generate_app()
+
+# Only run the App if this script is invoked from a Shell
+if __name__ == '__main__':
+app.debug = True
+app.run(host='0.0.0.0', port=8088)
+
+# Otherwise provide a variable called 'application' for mod_wsgi
+else:
--- End diff --
Where is this application var used? Its not global var as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35605766
--- Diff: reporter/usage-report-collector.py ---
@@ -0,0 +1,64 @@
+#!/usr/bin/env python
+# 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.
+
+from flask import abort, Flask, request, Response
+from elasticsearch import Elasticsearch
+import json
+import time
+
+def json_response(response):
+return json.dumps(response, indent=2) + "\n", 200, {'Content-Type':
'application/json; charset=utf-8'}
+
+def generate_app(config=None):
+app = Flask(__name__)
+
+@app.route('/report/', methods=['POST'])
+def report(unique_id):
+# We expect JSON data, so if the Content-Type doesn't match JSON
data we throw an error
+if 'Content-Type' in request.headers:
+if request.headers['Content-Type'] != 'application/json':
+abort(417, "No or incorrect Content-Type header was
supplied")
+
+index = "cloudstack-%s" % time.strftime("%Y.%m.%d", time.gmtime())
+timestamp = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime())
+
+es = Elasticsearch()
+es.indices.create(index=index, ignore=400)
+
+report = json.loads(request.data)
--- End diff --
May be we wanted to add a small check to see report is None and proceed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...
Github user sedukull commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/625#discussion_r35605842 --- Diff: reporter/usage-report-collector.py --- @@ -0,0 +1,64 @@ +#!/usr/bin/env python +# 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. + +from flask import abort, Flask, request, Response --- End diff -- This is good work i would say, but just add some comments on this service purpose, may be for people seeing it later can understand it easily. Any reason to have this service written in python and other usage code to run along with cs in java? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35606107
--- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
@@ -0,0 +1,487 @@
+// 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.cloudstack.report;
+
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.net.URL;
+import java.net.SocketTimeoutException;
+import java.net.MalformedURLException;
+import java.net.ProtocolException;
+import java.net.UnknownHostException;
+import java.io.OutputStreamWriter;
+import java.io.IOException;
+
+import javax.inject.Inject;
+import javax.net.ssl.HttpsURLConnection;
+
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+
+import org.apache.commons.codec.digest.DigestUtils;
+
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.dc.ClusterVO;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.component.ComponentMethodInterceptable;
+import com.cloud.utils.concurrency.NamedThreadFactory;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.upgrade.dao.VersionDao;
+import com.cloud.upgrade.dao.VersionVO;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.DiskOfferingVO;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.common.util.concurrent.AtomicLongMap;
+
+@Component
+public class UsageReporter extends ManagerBase implements
ComponentMethodInterceptable {
+public static final Logger s_logger =
Logger.getLogger(UsageReporter.class.getName());
+
+/* !FIX ME! This should point to a Apache Infra host with SSL! */
+private String reportHost = "https://call-home.cloudstack.org/report";;
+
+private String uniqueID = null;
+
+private static UsageReporter s_instance = null;
+
+private ScheduledExecutorService _executor = null;
+
+@Inject
+private ConfigurationDao _configDao;
+@Inject
+private HostDao _hostDao;
+@Inject
+private ClusterDao _clusterDao;
+@Inject
+private PrimaryDataStoreDao _storagePoolDao;
+@Inject
+private DataCenterDao _dataCenterDao;
+@Inject
+private UserVmDao _userVmDao;
+@Inject
+private VMInstanceDao _vmInstance;
+@Inject
+private VersionDao _versionDao;
+@Inject
+private DiskOfferingDao _diskOfferingDao;
+
+int usageReportInterval = -1;
+
+public static UsageReporter getInstance() {
+return s_instance;
+}
+
+public static UsageReporter getInstance(Map configs) {
+s_instance.init(configs);
+return s_instance;
+}
+
[GitHub] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/625#discussion_r35627322
--- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
@@ -0,0 +1,487 @@
+// 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.cloudstack.report;
+
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.net.URL;
+import java.net.SocketTimeoutException;
+import java.net.MalformedURLException;
+import java.net.ProtocolException;
+import java.net.UnknownHostException;
+import java.io.OutputStreamWriter;
+import java.io.IOException;
+
+import javax.inject.Inject;
+import javax.net.ssl.HttpsURLConnection;
+
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+
+import org.apache.commons.codec.digest.DigestUtils;
+
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.dc.ClusterVO;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.component.ComponentMethodInterceptable;
+import com.cloud.utils.concurrency.NamedThreadFactory;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.upgrade.dao.VersionDao;
+import com.cloud.upgrade.dao.VersionVO;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.DiskOfferingVO;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.common.util.concurrent.AtomicLongMap;
+
+@Component
+public class UsageReporter extends ManagerBase implements
ComponentMethodInterceptable {
+public static final Logger s_logger =
Logger.getLogger(UsageReporter.class.getName());
+
+/* !FIX ME! This should point to a Apache Infra host with SSL! */
+private String reportHost = "https://call-home.cloudstack.org/report";;
+
+private String uniqueID = null;
+
+private static UsageReporter s_instance = null;
+
+private ScheduledExecutorService _executor = null;
+
+@Inject
+private ConfigurationDao _configDao;
+@Inject
+private HostDao _hostDao;
+@Inject
+private ClusterDao _clusterDao;
+@Inject
+private PrimaryDataStoreDao _storagePoolDao;
+@Inject
+private DataCenterDao _dataCenterDao;
+@Inject
+private UserVmDao _userVmDao;
+@Inject
+private VMInstanceDao _vmInstance;
+@Inject
+private VersionDao _versionDao;
+@Inject
+private DiskOfferingDao _diskOfferingDao;
+
+int usageReportInterval = -1;
+
+public static UsageReporter getInstance() {
--- End diff --
It seems the purpose is to make only one instance of usagereporter
available per process and so we added getInstance() method, where user calls
the getInstance() to get the obje
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36608900
--- Diff:
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyFileInVmCommandWrapper.java
---
@@ -0,0 +1,59 @@
+/*
+ * 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 com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CopyFileInVmAnswer;
+import com.cloud.agent.api.CopyFileInVmCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.ssh.SshHelper;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.filefilter.TrueFileFilter;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+
+@ResourceWrapper(handles = CopyFileInVmCommand.class)
+public class LibvirtCopyFileInVmCommandWrapper extends
CommandWrapper {
+
+private static final Logger s_logger =
Logger.getLogger(LibvirtCopyFileInVmCommandWrapper.class);
+
+@Override public Answer execute(CopyFileInVmCommand cmd,
LibvirtComputingResource libvirtComputingResource) {
+final File keyFile = new File("/root/.ssh/id_rsa.cloud");
+try {
+File file = new File(cmd.getSrc());
--- End diff --
This code seems repetitive, can this be made as a lib, or convert to a
factory based upon given hypervisor type and use that pattern in each command
execute call?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36609019
--- Diff:
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
---
@@ -1279,6 +1285,27 @@ private static DiskTO getIsoDiskTO(DiskTO[] disks) {
return null;
}
+private CopyFileInVmAnswer execute(CopyFileInVmCommand cmd) {
+VmwareManager mgr =
getServiceContext().getStockObject(VmwareManager.CONTEXT_STOCK_NAME);
+File keyFile = mgr.getSystemVMKeyFile();
+try {
+File file = new File(cmd.getSrc());
--- End diff --
Does file object consumes a resource to be flushed or closed post the usage
or its auto closed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36609071
--- Diff:
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
---
@@ -1279,6 +1285,27 @@ private static DiskTO getIsoDiskTO(DiskTO[] disks) {
return null;
}
+private CopyFileInVmAnswer execute(CopyFileInVmCommand cmd) {
+VmwareManager mgr =
getServiceContext().getStockObject(VmwareManager.CONTEXT_STOCK_NAME);
+File keyFile = mgr.getSystemVMKeyFile();
+try {
+File file = new File(cmd.getSrc());
+if(file.exists()) {
+if(file.isDirectory()) {
+for (File f : FileUtils.listFiles(new
File(cmd.getSrc()), TrueFileFilter.INSTANCE, TrueFileFilter.INSTANCE)) {
--- End diff --
As well, here if its a directory, we are using multiple file objects, are
these auto closed and resources consumed are released back or does the api for
File..provides that facility?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36609212
--- Diff:
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyFileInVmCommandWrapper.java
---
@@ -0,0 +1,59 @@
+/*
+ * 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 com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CopyFileInVmAnswer;
+import com.cloud.agent.api.CopyFileInVmCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.ssh.SshHelper;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.filefilter.TrueFileFilter;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+
+@ResourceWrapper(handles = CopyFileInVmCommand.class)
+public class LibvirtCopyFileInVmCommandWrapper extends
CommandWrapper {
+
+private static final Logger s_logger =
Logger.getLogger(LibvirtCopyFileInVmCommandWrapper.class);
+
+@Override public Answer execute(CopyFileInVmCommand cmd,
LibvirtComputingResource libvirtComputingResource) {
+final File keyFile = new File("/root/.ssh/id_rsa.cloud");
+try {
+File file = new File(cmd.getSrc());
+if(file.exists()) {
+if(file.isDirectory()) {
+for (File f : FileUtils.listFiles(new
File(cmd.getSrc()), TrueFileFilter.INSTANCE, TrueFileFilter.INSTANCE)) {
+SshHelper.scpTo(cmd.getVmIp(), 3922, "root",
keyFile, null, cmd.getDest(), f.getCanonicalPath(), null);
+}
+} else {
--- End diff --
If its not a directory, but can be a block or socket file possible? Is
there any criteria check to be added for file type to be copied?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36609308
--- Diff:
plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/resource/HypervDirectConnectResource.java
---
@@ -595,6 +597,25 @@ private PlugNicAnswer execute(PlugNicCommand cmd) {
}
}
+private CopyFileInVmAnswer execute(CopyFileInVmCommand cmd) {
+File keyFile = getSystemVMKeyFile();
+try {
+File file = new File(cmd.getSrc());
+if(file.exists()) {
+if(file.isDirectory()) {
+for (File f : FileUtils.listFiles(new
File(cmd.getSrc()), TrueFileFilter.INSTANCE, TrueFileFilter.INSTANCE)) {
+SshHelper.scpTo(cmd.getVmIp(), 3922, "root",
keyFile, null, cmd.getDest(), f.getCanonicalPath(), null);
+}
+} else {
+SshHelper.scpTo(cmd.getVmIp(), 3922, "root", keyFile,
null, cmd.getDest(), file.getCanonicalPath(), null);
+}
+}
+} catch (Exception e) {
+s_logger.error("Fail to copy file " + cmd.getSrc() + " in VM "
+ cmd.getVmIp(), e);
+return new CopyFileInVmAnswer(cmd, e);
+}
+return new CopyFileInVmAnswer(cmd);
--- End diff --
Should this be moved inside to end of try block?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36611175
--- Diff: server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java ---
@@ -1425,19 +1423,39 @@ public boolean finalizeCommandsOnStart(Commands
cmds, VirtualMachineProfile prof
CheckSshCommand check = new
CheckSshCommand(profile.getInstanceName(), controlNic.getIp4Address(), 3922);
cmds.addCommand("checkSsh", check);
+try {
+File uiFiles = new File("systemvm/js");
+if (!uiFiles.exists()) {
+uiFiles = new
File("/usr/share/cloudstack-common/systemvm/js");
+}
+if (uiFiles.exists() && uiFiles.isDirectory()) {
+CopyFileInVmCommand copyFile = new
CopyFileInVmCommand(uiFiles.getCanonicalPath(), "/usr/local/cloud/systemvm/js",
controlNic.getIp4Address());
+cmds.addCommand("copyFile", copyFile);
+} else {
+s_logger.error("Couldn't locate localization files for
console proxy");
+return false;
+}
+} catch (IOException e) {
+s_logger.error("Failed to copy localization files for console
proxy: " + e.getMessage());
--- End diff --
should we return false in case of an exception?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36611238
--- Diff: server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java ---
@@ -1425,19 +1423,39 @@ public boolean finalizeCommandsOnStart(Commands
cmds, VirtualMachineProfile prof
CheckSshCommand check = new
CheckSshCommand(profile.getInstanceName(), controlNic.getIp4Address(), 3922);
cmds.addCommand("checkSsh", check);
+try {
+File uiFiles = new File("systemvm/js");
+if (!uiFiles.exists()) {
+uiFiles = new
File("/usr/share/cloudstack-common/systemvm/js");
+}
+if (uiFiles.exists() && uiFiles.isDirectory()) {
+CopyFileInVmCommand copyFile = new
CopyFileInVmCommand(uiFiles.getCanonicalPath(), "/usr/local/cloud/systemvm/js",
controlNic.getIp4Address());
+cmds.addCommand("copyFile", copyFile);
+} else {
+s_logger.error("Couldn't locate localization files for
console proxy");
+return false;
+}
+} catch (IOException e) {
+s_logger.error("Failed to copy localization files for console
proxy: " + e.getMessage());
+}
+
return true;
}
@Override
public boolean finalizeStart(VirtualMachineProfile profile, long
hostId, Commands cmds, ReservationContext context) {
-CheckSshAnswer answer = (CheckSshAnswer)cmds.getAnswer("checkSsh");
-if (answer == null || !answer.getResult()) {
-if (answer != null) {
-s_logger.warn("Unable to ssh to the VM: " +
answer.getDetails());
-} else {
-s_logger.warn("Unable to ssh to the VM: null answer");
+for(Answer answer : cmds.getAnswers()) {
+if(answer == null || !answer.getResult()) {
+if (answer != null) {
+s_logger.warn("Unable to ssh to the VM: " +
answer.getDetails());
+} else {
+s_logger.warn("Unable to ssh to the VM: null answer");
+}
+if(cmds.getAnswer("copyFile") == answer) {
+continue;
+}
+return false;
--- End diff --
It seems this return seems to be inside of if..block, so for loop
effectively has no effect and returns for one run?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36611512
--- Diff:
plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/resource/HypervDirectConnectResource.java
---
@@ -595,6 +597,25 @@ private PlugNicAnswer execute(PlugNicCommand cmd) {
}
}
+private CopyFileInVmAnswer execute(CopyFileInVmCommand cmd) {
+File keyFile = getSystemVMKeyFile();
+try {
+File file = new File(cmd.getSrc());
+if(file.exists()) {
+if(file.isDirectory()) {
+for (File f : FileUtils.listFiles(new
File(cmd.getSrc()), TrueFileFilter.INSTANCE, TrueFileFilter.INSTANCE)) {
+SshHelper.scpTo(cmd.getVmIp(), 3922, "root",
keyFile, null, cmd.getDest(), f.getCanonicalPath(), null);
+}
+} else {
+SshHelper.scpTo(cmd.getVmIp(), 3922, "root", keyFile,
null, cmd.getDest(), file.getCanonicalPath(), null);
+}
+}
+} catch (Exception e) {
+s_logger.error("Fail to copy file " + cmd.getSrc() + " in VM "
+ cmd.getVmIp(), e);
+return new CopyFileInVmAnswer(cmd, e);
+}
+return new CopyFileInVmAnswer(cmd);
--- End diff --
I mean in case a copyfilein..if moved inside try block throws an exception
, then the code return new...copyfile inside catch still works.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36611661
--- Diff:
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyFileInVmCommandWrapper.java
---
@@ -0,0 +1,59 @@
+/*
+ * 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 com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CopyFileInVmAnswer;
+import com.cloud.agent.api.CopyFileInVmCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.ssh.SshHelper;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.filefilter.TrueFileFilter;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+
+@ResourceWrapper(handles = CopyFileInVmCommand.class)
+public class LibvirtCopyFileInVmCommandWrapper extends
CommandWrapper {
+
+private static final Logger s_logger =
Logger.getLogger(LibvirtCopyFileInVmCommandWrapper.class);
+
+@Override public Answer execute(CopyFileInVmCommand cmd,
LibvirtComputingResource libvirtComputingResource) {
+final File keyFile = new File("/root/.ssh/id_rsa.cloud");
+try {
+File file = new File(cmd.getSrc());
+if(file.exists()) {
+if(file.isDirectory()) {
+for (File f : FileUtils.listFiles(new
File(cmd.getSrc()), TrueFileFilter.INSTANCE, TrueFileFilter.INSTANCE)) {
+SshHelper.scpTo(cmd.getVmIp(), 3922, "root",
keyFile, null, cmd.getDest(), f.getCanonicalPath(), null);
+}
+} else {
--- End diff --
I mean we are checking whether its directory or not, but if its not a
directory, other types can come EX: block, socket, or other file types defined
by attributes ( like d,b etc), i mean do we have a specific requirement to copy
with specific file type. EX: if we have group of files ( directory, block type,
socket type , plain text files etc) , with above logic it returns all which is
not of directory "type", iam inquiring to see we want to be explicit in code
for a given file type?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36704745
--- Diff: server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java ---
@@ -1425,19 +1423,39 @@ public boolean finalizeCommandsOnStart(Commands
cmds, VirtualMachineProfile prof
CheckSshCommand check = new
CheckSshCommand(profile.getInstanceName(), controlNic.getIp4Address(), 3922);
cmds.addCommand("checkSsh", check);
+try {
+File uiFiles = new File("systemvm/js");
+if (!uiFiles.exists()) {
+uiFiles = new
File("/usr/share/cloudstack-common/systemvm/js");
+}
+if (uiFiles.exists() && uiFiles.isDirectory()) {
+CopyFileInVmCommand copyFile = new
CopyFileInVmCommand(uiFiles.getCanonicalPath(), "/usr/local/cloud/systemvm/js",
controlNic.getIp4Address());
+cmds.addCommand("copyFile", copyFile);
+} else {
+s_logger.error("Couldn't locate localization files for
console proxy");
+return false;
+}
+} catch (IOException e) {
+s_logger.error("Failed to copy localization files for console
proxy: " + e.getMessage());
--- End diff --
My note is related to catch block, once an io exception is caught, do we
want to return false to callee ( inside the catch block)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36705033
--- Diff: systemvm/js/ajaxviewer.js ---
@@ -184,7 +205,7 @@ KeyboardMapper.prototype = {
if(eventType == AjaxViewer.KEY_UP && (code ==
AjaxViewer.JS_KEY_ALT || code == AjaxViewer.JS_KEY_CTRL))
this.mappedInput.push({type : eventType, code:
this.jsX11KeysymMap[code], modifiers: modifiers});
- } else if(eventType == AjaxViewer.KEY_PRESS && guestos ==
'null') {
+ } else if(eventType == AjaxViewer.KEY_PRESS &&
guestos.toLowerCase() != 'windows') {
--- End diff --
This might be existing code, but for more cleaner way, do we want to remote
all os strings and make them as constants or sort of enums defined at one place
and use them with reference here?
something like:
gustos.toLowerCase() != OsTypes.WINDOWS
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36705202
--- Diff:
plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/resource/HypervDirectConnectResource.java
---
@@ -595,6 +597,25 @@ private PlugNicAnswer execute(PlugNicCommand cmd) {
}
}
+private CopyFileInVmAnswer execute(CopyFileInVmCommand cmd) {
+File keyFile = getSystemVMKeyFile();
+try {
+File file = new File(cmd.getSrc());
+if(file.exists()) {
+if(file.isDirectory()) {
+for (File f : FileUtils.listFiles(new
File(cmd.getSrc()), TrueFileFilter.INSTANCE, TrueFileFilter.INSTANCE)) {
+SshHelper.scpTo(cmd.getVmIp(), 3922, "root",
keyFile, null, cmd.getDest(), f.getCanonicalPath(), null);
+}
+} else {
+SshHelper.scpTo(cmd.getVmIp(), 3922, "root", keyFile,
null, cmd.getDest(), file.getCanonicalPath(), null);
+}
+}
+} catch (Exception e) {
+s_logger.error("Fail to copy file " + cmd.getSrc() + " in VM "
+ cmd.getVmIp(), e);
+return new CopyFileInVmAnswer(cmd, e);
+}
+return new CopyFileInVmAnswer(cmd);
--- End diff --
Its not a big issue as such, but iam seeing like if there is an exception
at liine 617: "return new CopyFileIn.. (cmd)", then catch will still throw
your new object created. Again, its not a great issue to be worry about!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36705320
--- Diff:
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyFileInVmCommandWrapper.java
---
@@ -0,0 +1,59 @@
+/*
+ * 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 com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CopyFileInVmAnswer;
+import com.cloud.agent.api.CopyFileInVmCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.ssh.SshHelper;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.filefilter.TrueFileFilter;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+
+@ResourceWrapper(handles = CopyFileInVmCommand.class)
+public class LibvirtCopyFileInVmCommandWrapper extends
CommandWrapper {
+
+private static final Logger s_logger =
Logger.getLogger(LibvirtCopyFileInVmCommandWrapper.class);
+
+@Override public Answer execute(CopyFileInVmCommand cmd,
LibvirtComputingResource libvirtComputingResource) {
+final File keyFile = new File("/root/.ssh/id_rsa.cloud");
+try {
+File file = new File(cmd.getSrc());
--- End diff --
I mean the below code is used across few code files in same form or some
modified form, thats where i made a note, to avoid duplication. If its possible
to make one common definition and use across.
File file = new File(cmd.getSrc());
+if(file.exists()) {
+if(file.isDirectory()) {
+for(File f : FileUtils.listFiles(file,
TrueFileFilter.INSTANCE, TrueFileFilter.INSTANCE)) {
+ExecutionResult result =
citrixResourceBase.copyFileInVm(cmd.getVmIp(), f, cmd.getDest());
+if(!result.isSuccess()) {
+return new CopyFileInVmAnswer(cmd,
result.getDetails());
+}
+}
+} else {
+ExecutionResult result =
citrixResourceBase.copyFileInVm(cmd.getVmIp(), file, cmd.getDest());
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/669#discussion_r36705867
--- Diff:
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyFileInVmCommandWrapper.java
---
@@ -0,0 +1,59 @@
+/*
+ * 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 com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CopyFileInVmAnswer;
+import com.cloud.agent.api.CopyFileInVmCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.ssh.SshHelper;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.filefilter.TrueFileFilter;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+
+@ResourceWrapper(handles = CopyFileInVmCommand.class)
+public class LibvirtCopyFileInVmCommandWrapper extends
CommandWrapper {
+
+private static final Logger s_logger =
Logger.getLogger(LibvirtCopyFileInVmCommandWrapper.class);
+
+@Override public Answer execute(CopyFileInVmCommand cmd,
LibvirtComputingResource libvirtComputingResource) {
+final File keyFile = new File("/root/.ssh/id_rsa.cloud");
+try {
+File file = new File(cmd.getSrc());
+if(file.exists()) {
+if(file.isDirectory()) {
+for (File f : FileUtils.listFiles(new
File(cmd.getSrc()), TrueFileFilter.INSTANCE, TrueFileFilter.INSTANCE)) {
+SshHelper.scpTo(cmd.getVmIp(), 3922, "root",
keyFile, null, cmd.getDest(), f.getCanonicalPath(), null);
+}
+} else {
--- End diff --
My note was related to the below example.
example_dir_for_iteration contains say 5 entries as follows (dira,
text_file_a,socket_file_a,block_device_file_a,zip_file_a), in else block of the
code, we are copying all files. Now, in your case where you explicitly
mentioned key file path(/root/..) these file types may not be possible, but
these type of copying without file types checks, leads to vulnerabilities in
the application long run. So, i mentioned to see we are copying files as
mentioned in else block, do we want to add some file type check that we are
interesting in copying, for that i gave an example where directory contains
multiple file types.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/669#discussion_r36832023 --- Diff: client/pom.xml --- @@ -478,6 +478,12 @@ + --- End diff -- Do we have any unit-tests added for this change? As well, if we possible, can we have a document for the console proxy workflow available on wiki ? This may help people to know the exact workflow or class interactions with business flow. As well, a small change to this request message "Made the adding new keyboard language support easier" could be beneficial i believe, helps people to know the complete context for this change. Note :this is not specific to this line, but a generic note. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/669#discussion_r36839282 --- Diff: client/pom.xml --- @@ -478,6 +478,12 @@ + --- End diff -- Thanks for the FS link, it was helpful. Regarding the unit test, i would disagree to say that they are meant to test corner cases. Not sure, what it doesnt make sense to you. In general, unit test purpose is not what you meant, in this commit, if we see we modified near to 28 code files, added new classes and code, it makes sense to have unit tests for the added code. It will help to test the code units and individual functional blocks during build, these areas not always are touched by functional tests. It adds to the coverage, make sure code modified down the lane adheres to the contract through failures in tests etc. Its appreciated, if we can have few unit tests added to this, right now they are zero tests for this change. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8727: API call listVirtualMach...
Github user sedukull commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/685#discussion_r36855234 --- Diff: setup/db/db/schema-452to460.sql --- @@ -353,6 +353,8 @@ CREATE VIEW `cloud`.`user_vm_view` AS `cloud`.`user_vm_details` `custom_speed` ON (((`custom_speed`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_speed`.`name` = 'CpuSpeed'))) left join `cloud`.`user_vm_details` `custom_ram_size` ON (((`custom_ram_size`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_ram_size`.`name` = 'memory'))); +delete s1 from ssh_keypairs s1, ssh_keypairs s2 where s1.id > s2.id and s1.public_key = s2.public_key; --- End diff -- I believe, its an api change required rather adding the constraint at the db level, otherwise it will mask the actual issue in the code where these duplicates are getting added. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8727: API call listVirtualMach...
Github user sedukull commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/685#discussion_r36855422 --- Diff: setup/db/db/schema-452to460.sql --- @@ -353,6 +353,8 @@ CREATE VIEW `cloud`.`user_vm_view` AS `cloud`.`user_vm_details` `custom_speed` ON (((`custom_speed`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_speed`.`name` = 'CpuSpeed'))) left join `cloud`.`user_vm_details` `custom_ram_size` ON (((`custom_ram_size`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_ram_size`.`name` = 'memory'))); +delete s1 from ssh_keypairs s1, ssh_keypairs s2 where s1.id > s2.id and s1.public_key = s2.public_key; +ALTER TABLE ssh_keypairs ADD UNIQUE (fingerprint); --- End diff -- Here, we added unique constraint on fingerprint column, but in previous one you deleted the rows where public_key column values are same. The bug mentions that "previous" ones added with same names are deleted, here it seems "id" is an auto increment column and is deleting rows which are added "later", not the "previous" ones. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8727: API call listVirtualMach...
Github user sedukull commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/685#discussion_r36855515 --- Diff: setup/db/db/schema-452to460.sql --- @@ -353,6 +353,8 @@ CREATE VIEW `cloud`.`user_vm_view` AS `cloud`.`user_vm_details` `custom_speed` ON (((`custom_speed`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_speed`.`name` = 'CpuSpeed'))) left join `cloud`.`user_vm_details` `custom_ram_size` ON (((`custom_ram_size`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_ram_size`.`name` = 'memory'))); +delete s1 from ssh_keypairs s1, ssh_keypairs s2 where s1.id > s2.id and s1.public_key = s2.public_key; +ALTER TABLE ssh_keypairs ADD UNIQUE (fingerprint); --- End diff -- Can we indent these two lines better, currently it shows as part of view creation? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8727: API call listVirtualMach...
Github user sedukull commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/685#discussion_r36855563 --- Diff: setup/db/db/schema-452to460.sql --- @@ -353,6 +353,8 @@ CREATE VIEW `cloud`.`user_vm_view` AS `cloud`.`user_vm_details` `custom_speed` ON (((`custom_speed`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_speed`.`name` = 'CpuSpeed'))) left join `cloud`.`user_vm_details` `custom_ram_size` ON (((`custom_ram_size`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_ram_size`.`name` = 'memory'))); +delete s1 from ssh_keypairs s1, ssh_keypairs s2 where s1.id > s2.id and s1.public_key = s2.public_key; +ALTER TABLE ssh_keypairs ADD UNIQUE (fingerprint); --- End diff -- As well, there was a discussion on the mailing list that all DB changes be discussed on the ML. Can we discuss this issue on ML first? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8727: API call listVirtualMach...
Github user sedukull commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/685#discussion_r36857039 --- Diff: setup/db/db/schema-452to460.sql --- @@ -353,6 +353,8 @@ CREATE VIEW `cloud`.`user_vm_view` AS `cloud`.`user_vm_details` `custom_speed` ON (((`custom_speed`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_speed`.`name` = 'CpuSpeed'))) left join `cloud`.`user_vm_details` `custom_ram_size` ON (((`custom_ram_size`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_ram_size`.`name` = 'memory'))); +delete s1 from ssh_keypairs s1, ssh_keypairs s2 where s1.id > s2.id and s1.public_key = s2.public_key; --- End diff -- It seems ssh_keypairs table, has foreign key constraints with cascading delete logic, as per table definition. Will these delete statements effect the rows in other tables say account and domain? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Made the adding new keyboard language sup...
Github user sedukull commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/669#discussion_r36952117 --- Diff: client/pom.xml --- @@ -478,6 +478,12 @@ + --- End diff -- Its good that it does not say that UT are meant to test "corner" cases or "some uncertainity", as per your earlier understanding. It talks about TDD, in which UT are not meant to be written post the code, its good to follow TDD if we can write tests before the code. But if we missed it, and If we feel there is a value add in writing some tests, which can benefit others and helps testing the code in long run, its appreciated to add few, there is no harm. We could see many commits coming are having UT recently. Some even made sure to get near 100% coverage for their code. We know its not a new distinct module, its up to you and whatever the community decides!!! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack-www pull request: Python script to generate pmc and com...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack-www/pull/14#discussion_r38068341
--- Diff: makewholist.py ---
@@ -0,0 +1,68 @@
+#!/usr/bin/env python
+# 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.
+
+import json
+import sys
+import urllib
+
+groups_url = "https://projects.apache.org/json/foundation/groups.json";
+names_url = "https://projects.apache.org/json/foundation/people_name.json";
+
+names = {} # Placeholder for names parsed in getNames()
+groups = {} # Placeholder for groups parsed in getCommittersAndPMC()
+
+def getCommittersAndPMC():
+global groups
+try:
+response = urllib.urlopen(groups_url)
--- End diff --
mention global for groups_url as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack-www pull request: Python script to generate pmc and com...
Github user sedukull commented on a diff in the pull request:
https://github.com/apache/cloudstack-www/pull/14#discussion_r38068366
--- Diff: makewholist.py ---
@@ -0,0 +1,68 @@
+#!/usr/bin/env python
+# 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.
+
+import json
+import sys
+import urllib
+
+groups_url = "https://projects.apache.org/json/foundation/groups.json";
+names_url = "https://projects.apache.org/json/foundation/people_name.json";
+
+names = {} # Placeholder for names parsed in getNames()
+groups = {} # Placeholder for groups parsed in getCommittersAndPMC()
+
+def getCommittersAndPMC():
+global groups
+try:
+response = urllib.urlopen(groups_url)
+groups = json.loads(response.read())
+except Exception, e:
+print "Error during groups JSON download or parsing: %s" %
e.message
+
+def getNames():
+global names
+try:
+response = urllib.urlopen(names_url)
--- End diff --
use global names_url here as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack-www pull request: Python script to generate pmc and com...
Github user sedukull commented on the pull request: https://github.com/apache/cloudstack-www/pull/14#issuecomment-135375292 Was looking to be consistent when referring to global variables, but otherwise LGTM. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8727: API call listVirtualMach...
Github user sedukull commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/685#discussion_r38192659 --- Diff: setup/db/db/schema-452to460.sql --- @@ -354,6 +354,10 @@ CREATE VIEW `cloud`.`user_vm_view` AS left join `cloud`.`user_vm_details` `custom_ram_size` ON (((`custom_ram_size`.`vm_id` = `cloud`.`vm_instance`.`id`) and (`custom_ram_size`.`name` = 'memory'))); +---Additional checks to ensure duplicate keys are not registered and remove the previously stored duplicate keys. +DELETE `s1` FROM `ssh_keypairs` `s1`, `ssh_keypairs` `s2` WHERE `s1`.`id` > `s2`.`id` AND `s1`.`public_key` = `s2`.`public_key` AND `s1`.`account_id` = `s2`.`account_id`; +ALTER TABLE `ssh_keypairs` ADD UNIQUE `unique_index`(`fingerprint`,`account_id`); --- End diff -- May be name unique_index can be better renamed? By default UNIQUE constraint signifies that. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixing findbugs error due to PR #755 merg...
Github user sedukull commented on the pull request: https://github.com/apache/cloudstack/pull/779#issuecomment-138193843 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
