unity-scope-click

Merge lp:~dobey/unity-scope-click/base-url-env into lp:unity-scope-click/devel

  1. base-url-env
  2. Merge into devel
Proposed by Alejandro J. Cura
Status: Merged
Approved by: Alejandro J. Cura
Approved revision: 212
Merged at revision: 216
Proposed branch: lp:~dobey/unity-scope-click/base-url-env
Merge into: lp:unity-scope-click/devel
Prerequisite: lp:~stolowski/unity-scope-click/remove-url-dispatcher
Diff against target: 130 lines (+36/-5)
5 files modified
scope/click/index.cpp (+12/-2)
scope/click/index.h (+3/-0)
scope/click/reviews.cpp (+2/-2)
scope/click/reviews.h (+1/-1)
scope/tests/test_index.cpp (+18/-0)
To merge this branch: bzr merge lp:~dobey/unity-scope-click/base-url-env
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alejandro J. Cura (community) Approve
Paweł Stołowski Pending
Michal Hruby Pending
Review via email: mp+217065@code.launchpad.net

This proposal supersedes a proposal from 2014-04-16.

Commit message

Add support for getting search index URL from env.
Fix the naming of the environment variable for reviews base URL.
Use cstdlib instead of stdlib.h for the include.

Description of the change

Add support for getting search index URL from env.
Fix the naming of the environment variable for reviews base URL.
Use cstdlib instead of stdlib.h for the include.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:210
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/424/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-amd64-ci/325
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/322
        deb: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/322/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/424/rebuild

review: Approve (continuous-integration)
Revision history for this message
Paweł Stołowski (stolowski) wrote : Posted in a previous version of this proposal

It doesn't cover all aspects of server communcation - in click::Query::run(..) there is still a hardcoded url:

qt::core::world::enter_with_task([=](qt::core::world::Environment& env)
    {
        static const QString queryPattern(
                    "https://search.apps.ubuntu.com/api/v1/search?q=%1"
                    "%2,architecture:%3");

review: Needs Fixing
Revision history for this message
Michal Hruby (mhr3) wrote : Posted in a previous version of this proposal

How do you plan to use this? If it's supposed to be used outside of tests, you'd ultimately need to modify scope-registry's environment for this to propagate to the scope.

review: Needs Information
Revision history for this message
dobey (dobey) wrote : Posted in a previous version of this proposal

> How do you plan to use this? If it's supposed to be used outside of tests,
> you'd ultimately need to modify scope-registry's environment for this to
> propagate to the scope.

It's mainly for use by the autopilot tests, so the tests can be run against a local server instance. The autopilot tests are modifying the environment and running the scope directly.

Revision history for this message
dobey (dobey) wrote : Posted in a previous version of this proposal

> It doesn't cover all aspects of server communcation - in click::Query::run(..)
> there is still a hardcoded url:
>
> qt::core::world::enter_with_task([=](qt::core::world::Environment& env)
> {
> static const QString queryPattern(
> "https://search.apps.ubuntu.com/api/v1/search?q=%1"
> "%2,architecture:%3");

Ugh. I forgot about the evil Query code. Fixed it now too.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:211
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/427/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-amd64-ci/328
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/325
        deb: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/325/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/427/rebuild

review: Approve (continuous-integration)
Revision history for this message
Paweł Stołowski (stolowski) wrote : Posted in a previous version of this proposal

Looks good now. +1

review: Approve
Revision history for this message
Michal Hruby (mhr3) wrote : Posted in a previous version of this proposal

As Marcus said, manually starting a scope should be supported, so +1 from me.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:212
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/435/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-amd64-ci/336
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/333
        deb: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/333/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/435/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alejandro J. Cura (alecu) wrote : Posted in a previous version of this proposal

Resubmit looks good.

review: Approve
Revision history for this message
Alejandro J. Cura (alecu) :
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'scope/click/index.cpp'
2--- scope/click/index.cpp 2014-04-12 22:25:12 +0000
3+++ scope/click/index.cpp 2014-04-24 14:53:55 +0000
4@@ -31,6 +31,7 @@
5 #include <QObject>
6 #include <QProcess>
7
8+#include <cstdlib>
9 #include <json/json.h>
10 #include <sstream>
11
12@@ -305,7 +306,7 @@
13 std::string built_query(build_index_query(query));
14 params.add(click::QUERY_ARGNAME, built_query.c_str());
15 QSharedPointer<click::web::Response> response(client->call(
16- click::SEARCH_BASE_URL + click::SEARCH_PATH, params));
17+ get_base_url() + click::SEARCH_PATH, params));
18
19 QObject::connect(response.data(), &click::web::Response::finished, [=](QString reply) {
20 click::PackageList pl = click::package_list_from_json(reply.toUtf8().constData());
21@@ -325,7 +326,7 @@
22 click::web::Cancellable Index::get_details (const std::string& package_name, std::function<void(PackageDetails, click::Index::Error)> callback)
23 {
24 QSharedPointer<click::web::Response> response = client->call
25- (click::SEARCH_BASE_URL + click::DETAILS_PATH + package_name);
26+ (get_base_url() + click::DETAILS_PATH + package_name);
27 qDebug() << "getting details for" << package_name.c_str();
28
29 QObject::connect(response.data(), &click::web::Response::finished, [=](const QByteArray reply) {
30@@ -342,6 +343,15 @@
31 return click::web::Cancellable(response);
32 }
33
34+std::string Index::get_base_url ()
35+{
36+ const char *env_url = getenv(SEARCH_BASE_URL_ENVVAR.c_str());
37+ if (env_url != NULL) {
38+ return env_url;
39+ }
40+ return click::SEARCH_BASE_URL;
41+}
42+
43 Index::~Index()
44 {
45 }
46
47=== modified file 'scope/click/index.h'
48--- scope/click/index.h 2014-04-12 21:06:45 +0000
49+++ scope/click/index.h 2014-04-24 14:53:55 +0000
50@@ -47,6 +47,7 @@
51
52 class Configuration;
53
54+const std::string SEARCH_BASE_URL_ENVVAR = "U1_SEARCH_BASE_URL";
55 const std::string SEARCH_BASE_URL = "https://search.apps.ubuntu.com/";
56 const std::string SEARCH_PATH = "api/v1/search";
57 const std::string SUPPORTED_FRAMEWORKS = "framework:ubuntu-sdk-13.10";
58@@ -163,6 +164,8 @@
59 virtual click::web::Cancellable search (const std::string& query, std::function<void(PackageList)> callback);
60 virtual click::web::Cancellable get_details(const std::string& package_name, std::function<void(PackageDetails, Error)> callback);
61 virtual ~Index();
62+
63+ static std::string get_base_url ();
64 };
65
66 bool operator==(const Package& lhs, const Package& rhs);
67
68=== modified file 'scope/click/reviews.cpp'
69--- scope/click/reviews.cpp 2014-04-04 09:05:24 +0000
70+++ scope/click/reviews.cpp 2014-04-24 14:53:55 +0000
71@@ -27,7 +27,7 @@
72 * files in the program, then also delete it here.
73 */
74
75-#include <stdlib.h>
76+#include <cstdlib>
77
78 #include <boost/property_tree/ptree.hpp>
79 #include <boost/property_tree/json_parser.hpp>
80@@ -127,7 +127,7 @@
81 {
82 const char *env_url = getenv(REVIEWS_BASE_URL_ENVVAR.c_str());
83 if (env_url != NULL) {
84- return env_url;;
85+ return env_url;
86 }
87 return click::REVIEWS_BASE_URL;
88 }
89
90=== modified file 'scope/click/reviews.h'
91--- scope/click/reviews.h 2014-03-27 15:03:56 +0000
92+++ scope/click/reviews.h 2014-04-24 14:53:55 +0000
93@@ -40,7 +40,7 @@
94 namespace click
95 {
96
97-const std::string REVIEWS_BASE_URL_ENVVAR = "CLICK_REVIEWS_BASE_URL";
98+const std::string REVIEWS_BASE_URL_ENVVAR = "U1_REVIEWS_BASE_URL";
99 const std::string REVIEWS_BASE_URL = "https://reviews.ubuntu.com";
100 const std::string REVIEWS_API_PATH = "/click/api/1.0/reviews/";
101 const std::string REVIEWS_QUERY_ARGNAME = "package_name";
102
103=== modified file 'scope/tests/test_index.cpp'
104--- scope/tests/test_index.cpp 2014-04-12 22:25:12 +0000
105+++ scope/tests/test_index.cpp 2014-04-24 14:53:55 +0000
106@@ -421,6 +421,24 @@
107 get_details_operation.cancel();
108 }
109
110+TEST_F(IndexTest, testGetBaseUrl)
111+{
112+ const char *value = getenv(click::SEARCH_BASE_URL_ENVVAR.c_str());
113+ if (value != NULL) {
114+ ASSERT_TRUE(unsetenv(click::SEARCH_BASE_URL_ENVVAR.c_str()) == 0);
115+ }
116+ ASSERT_TRUE(click::Index::get_base_url() == click::SEARCH_BASE_URL);
117+
118+}
119+
120+TEST_F(IndexTest, testGetBaseUrlFromEnv)
121+{
122+ ASSERT_TRUE(setenv(click::SEARCH_BASE_URL_ENVVAR.c_str(),
123+ FAKE_SERVER.c_str(), 1) == 0);
124+ ASSERT_TRUE(click::Index::get_base_url() == FAKE_SERVER);
125+ ASSERT_TRUE(unsetenv(click::SEARCH_BASE_URL_ENVVAR.c_str()) == 0);
126+}
127+
128 TEST_F(MockPackageManager, testUninstallCommandCorrect)
129 {
130 click::Package package = {

Subscribers

People subscribed via source and target branches

to all changes: