Joris,
It breaks the build:
picojson-1.3.0/picojson.h: In member function ‘std::string
picojson::value::to_str() const’:
picojson-1.3.0/picojson.h:370:38: error: expected ‘)’ before ‘PRId64’
SNPRINTF(buf, sizeof(buf), "%" PRId64, u_.int64_);
On Wed, Sep 16, 2015 at 2:56 PM, <[email protected]> wrote:
> Integer Precision for JSON <-> Protobuf conversions.
>
> * Add TODO's for refactoring some JSON parsing in the docker code
> (See MESOS-3409). Update how the JSON::Number is used.
> * Tweak some tests to match changes to JSON::Number.
> * Address a TODO on one test, which used a workaround for
> double-precision comparison.
>
> Review: https://reviews.apache.org/r/38077
>
>
> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2c277f1c
> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2c277f1c
> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2c277f1c
>
> Branch: refs/heads/master
> Commit: 2c277f1c0e0dc0a6618ba930bb5f8d9dd753d4be
> Parents: df9eacb
> Author: Joseph Wu <[email protected]>
> Authored: Wed Sep 16 13:44:47 2015 -0400
> Committer: Joris Van Remoortere <[email protected]>
> Committed: Wed Sep 16 17:48:43 2015 -0400
>
> ----------------------------------------------------------------------
> src/docker/docker.cpp | 3 +-
> .../provisioners/docker/token_manager.cpp | 3 +-
> src/tests/fault_tolerance_tests.cpp | 2 +-
> src/tests/master_tests.cpp | 2 +-
> src/tests/monitor_tests.cpp | 50 +++------
> src/tests/rate_limiting_tests.cpp | 106 +++++++++++++------
> src/tests/slave_tests.cpp | 2 +-
> 7 files changed, 93 insertions(+), 75 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/2c277f1c/src/docker/docker.cpp
> ----------------------------------------------------------------------
> diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp
> index c4c37cb..afcedf1 100755
> --- a/src/docker/docker.cpp
> +++ b/src/docker/docker.cpp
> @@ -236,6 +236,7 @@ Try<Nothing> Docker::validateVersion(const Version&
> minVersion) const
> }
>
>
> +// TODO(josephw): Parse this string with a protobuf.
> Try<Docker::Container> Docker::Container::create(const string& output)
> {
> Try<JSON::Array> parse = JSON::parse<JSON::Array>(output);
> @@ -286,7 +287,7 @@ Try<Docker::Container> Docker::Container::create(const
> string& output)
> return Error("Error finding Pid in State: " + pidValue.error());
> }
>
> - pid_t pid = pid_t(pidValue.get().value);
> + pid_t pid = pid_t(pidValue.get().as<int64_t>());
>
> Option<pid_t> optionalPid;
> if (pid != 0) {
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/2c277f1c/src/slave/containerizer/provisioners/docker/token_manager.cpp
> ----------------------------------------------------------------------
> diff --git a/src/slave/containerizer/provisioners/docker/token_manager.cpp
> b/src/slave/containerizer/provisioners/docker/token_manager.cpp
> index aec915f..95f459d 100644
> --- a/src/slave/containerizer/provisioners/docker/token_manager.cpp
> +++ b/src/slave/containerizer/provisioners/docker/token_manager.cpp
> @@ -122,6 +122,7 @@ Token::Token(
> notBefore(_notBefore) {}
>
>
> +// TODO(josephw): Parse this string with some protobufs.
> Try<Token> Token::create(const string& raw)
> {
> auto decode = [](
> @@ -196,7 +197,7 @@ Result<Time> Token::getTimeValue(const JSON::Object&
> object, const string& key)
>
> // If expiration is provided, we will process it for future validations.
> if (jsonValue.isSome()) {
> - Try<Time> time = Time::create(jsonValue.get().value);
> + Try<Time> time = Time::create(jsonValue.get().as<double>());
> if (time.isError()) {
> return Error("Failed to decode time: " + time.error());
> }
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/2c277f1c/src/tests/fault_tolerance_tests.cpp
> ----------------------------------------------------------------------
> diff --git a/src/tests/fault_tolerance_tests.cpp
> b/src/tests/fault_tolerance_tests.cpp
> index 061e099..c97bc46 100644
> --- a/src/tests/fault_tolerance_tests.cpp
> +++ b/src/tests/fault_tolerance_tests.cpp
> @@ -1918,7 +1918,7 @@ TEST_F(FaultToleranceTest,
> UpdateFrameworkInfoOnSchedulerFailover)
> EXPECT_EQ(1u, framework.values.count("failover_timeout"));
> JSON::Number failoverTimeout =
> framework.values["failover_timeout"].as<JSON::Number>();
> - EXPECT_EQ(finfo2.failover_timeout(), failoverTimeout.value);
> + EXPECT_EQ(finfo2.failover_timeout(), failoverTimeout.as<double>());
>
> EXPECT_EQ(1u, framework.values.count("hostname"));
> JSON::String hostname = framework.values["hostname"].as<JSON::String>();
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/2c277f1c/src/tests/master_tests.cpp
> ----------------------------------------------------------------------
> diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
> index dd65fcc..e4c89ab 100644
> --- a/src/tests/master_tests.cpp
> +++ b/src/tests/master_tests.cpp
> @@ -2826,7 +2826,7 @@ TEST_F(MasterTest, StateEndpoint)
> ASSERT_TRUE(state.values["start_time"].is<JSON::Number>());
> EXPECT_EQ(
> static_cast<int>(Clock::now().secs()),
> -
> static_cast<int>(state.values["start_time"].as<JSON::Number>().value));
> + state.values["start_time"].as<JSON::Number>().as<int>());
>
> ASSERT_TRUE(state.values["id"].is<JSON::String>());
> EXPECT_NE("", state.values["id"].as<JSON::String>().value);
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/2c277f1c/src/tests/monitor_tests.cpp
> ----------------------------------------------------------------------
> diff --git a/src/tests/monitor_tests.cpp b/src/tests/monitor_tests.cpp
> index f404955..583e711 100644
> --- a/src/tests/monitor_tests.cpp
> +++ b/src/tests/monitor_tests.cpp
> @@ -106,44 +106,18 @@ TEST(MonitorTest, Statistics)
> "Content-Type",
> response);
>
> - // TODO(bmahler): Use JSON equality instead to avoid having to use
> - // numeric limits for double precision.
> - AWAIT_EXPECT_RESPONSE_BODY_EQ(
> - strings::format(
> - "[{"
> - "\"executor_id\":\"executor\","
> - "\"executor_name\":\"name\","
> - "\"framework_id\":\"framework\","
> - "\"source\":\"source\","
> - "\"statistics\":{"
> - "\"cpus_limit\":%g,"
> - "\"cpus_nr_periods\":%d,"
> - "\"cpus_nr_throttled\":%d,"
> - "\"cpus_system_time_secs\":%g,"
> - "\"cpus_throttled_time_secs\":%g,"
> - "\"cpus_user_time_secs\":%g,"
> - "\"mem_anon_bytes\":%lu,"
> - "\"mem_file_bytes\":%lu,"
> - "\"mem_limit_bytes\":%lu,"
> - "\"mem_mapped_file_bytes\":%lu,"
> - "\"mem_rss_bytes\":%lu,"
> - "\"timestamp\":"
> - "%." + stringify(numeric_limits<double>::digits10)
> + "g"
> - "}"
> - "}]",
> - statistics.cpus_limit(),
> - statistics.cpus_nr_periods(),
> - statistics.cpus_nr_throttled(),
> - statistics.cpus_system_time_secs(),
> - statistics.cpus_throttled_time_secs(),
> - statistics.cpus_user_time_secs(),
> - statistics.mem_anon_bytes(),
> - statistics.mem_file_bytes(),
> - statistics.mem_limit_bytes(),
> - statistics.mem_mapped_file_bytes(),
> - statistics.mem_rss_bytes(),
> - statistics.timestamp()).get(),
> - response);
> + JSON::Array expected;
> + JSON::Object usage;
> + usage.values["executor_id"] = "executor";
> + usage.values["executor_name"] = "name";
> + usage.values["framework_id"] = "framework";
> + usage.values["source"] = "source";
> + usage.values["statistics"] = JSON::Protobuf(statistics);
> + expected.values.push_back(usage);
> +
> + Try<JSON::Array> result = JSON::parse<JSON::Array>(response.get().body);
> + ASSERT_SOME(result);
> + ASSERT_EQ(expected, result.get());
> }
>
>
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/2c277f1c/src/tests/rate_limiting_tests.cpp
> ----------------------------------------------------------------------
> diff --git a/src/tests/rate_limiting_tests.cpp
> b/src/tests/rate_limiting_tests.cpp
> index f3aedde..e512aa6 100644
> --- a/src/tests/rate_limiting_tests.cpp
> +++ b/src/tests/rate_limiting_tests.cpp
> @@ -166,12 +166,16 @@ TEST_F(RateLimitingTest, NoRateLimiting)
> const string& messages_received =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_received";
> EXPECT_EQ(1u, metrics.values.count(messages_received));
> - EXPECT_EQ(1,
> metrics.values[messages_received].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
>
> const string& messages_processed =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_processed";
> EXPECT_EQ(1u, metrics.values.count(messages_processed));
> - EXPECT_EQ(1,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
> }
>
> Future<Nothing> removeFramework =
> @@ -270,12 +274,16 @@ TEST_F(RateLimitingTest, RateLimitingEnabled)
> const string& messages_received =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_received";
> EXPECT_EQ(1u, metrics.values.count(messages_received));
> - EXPECT_EQ(1,
> metrics.values[messages_received].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
>
> const string& messages_processed =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_processed";
> EXPECT_EQ(1u, metrics.values.count(messages_processed));
> - EXPECT_EQ(1,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
> }
>
> // The 2nd message is throttled for a second.
> @@ -305,8 +313,12 @@ TEST_F(RateLimitingTest, RateLimitingEnabled)
>
> // The 2nd message is received and but not processed after half
> // a second because of throttling.
> - EXPECT_EQ(2,
> metrics.values[messages_received].as<JSON::Number>().value);
> - EXPECT_EQ(1,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 2,
> +
> metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
> EXPECT_TRUE(duplicateFrameworkRegisteredMessage.isPending());
> }
>
> @@ -324,8 +336,10 @@ TEST_F(RateLimitingTest, RateLimitingEnabled)
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_processed";
> EXPECT_EQ(1u, metrics.values.count(messages_processed));
>
> - EXPECT_EQ(2,
> metrics.values[messages_received].as<JSON::Number>().value);
> - EXPECT_EQ(2,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 2,
> metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
> + EXPECT_EQ(
> + 2,
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
>
> EXPECT_EQ(DRIVER_STOPPED, driver.stop());
> EXPECT_EQ(DRIVER_STOPPED, driver.join());
> @@ -481,19 +495,19 @@ TEST_F(RateLimitingTest,
> DifferentPrincipalFrameworks)
> EXPECT_EQ(
> 2,
> metrics.values["frameworks/framework1/messages_received"]
> - .as<JSON::Number>().value);
> + .as<JSON::Number>().as<int64_t>());
> EXPECT_EQ(
> 2,
> metrics.values["frameworks/framework2/messages_received"]
> - .as<JSON::Number>().value);
> + .as<JSON::Number>().as<int64_t>());
> EXPECT_EQ(
> 1,
> metrics.values["frameworks/framework1/messages_processed"]
> - .as<JSON::Number>().value);
> + .as<JSON::Number>().as<int64_t>());
> EXPECT_EQ(
> 1,
> metrics.values["frameworks/framework2/messages_processed"]
> - .as<JSON::Number>().value);
> + .as<JSON::Number>().as<int64_t>());
> }
>
> // Advance for a second so the message from framework1 (1qps)
> @@ -508,11 +522,11 @@ TEST_F(RateLimitingTest,
> DifferentPrincipalFrameworks)
> EXPECT_EQ(
> 2,
> metrics.values["frameworks/framework1/messages_processed"]
> - .as<JSON::Number>().value);
> + .as<JSON::Number>().as<int64_t>());
> EXPECT_EQ(
> 1,
> metrics.values["frameworks/framework2/messages_processed"]
> - .as<JSON::Number>().value);
> + .as<JSON::Number>().as<int64_t>());
>
> // After another half a second framework2 (0.2qps)'s message is
> // processed as well.
> @@ -535,19 +549,19 @@ TEST_F(RateLimitingTest,
> DifferentPrincipalFrameworks)
> EXPECT_EQ(
> 2,
> metrics.values["frameworks/framework1/messages_received"]
> - .as<JSON::Number>().value);
> + .as<JSON::Number>().as<int64_t>());
> EXPECT_EQ(
> 2,
> metrics.values["frameworks/framework2/messages_received"]
> - .as<JSON::Number>().value);
> + .as<JSON::Number>().as<int64_t>());
> EXPECT_EQ(
> 2,
> metrics.values["frameworks/framework1/messages_processed"]
> - .as<JSON::Number>().value);
> + .as<JSON::Number>().as<int64_t>());
> EXPECT_EQ(
> 2,
> metrics.values["frameworks/framework2/messages_processed"]
> - .as<JSON::Number>().value);
> + .as<JSON::Number>().as<int64_t>());
> }
>
> // 3. Remove a framework and its message counters are deleted while
> @@ -705,12 +719,16 @@ TEST_F(RateLimitingTest, SamePrincipalFrameworks)
> const string& messages_received =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_received";
> EXPECT_EQ(1u, metrics.values.count(messages_received));
> - EXPECT_EQ(2,
> metrics.values[messages_received].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 2,
> +
> metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
>
> const string& messages_processed =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_processed";
> EXPECT_EQ(1u, metrics.values.count(messages_processed));
> - EXPECT_EQ(1,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
> }
>
> // Advance for another half a second to make sure throttled
> @@ -828,12 +846,16 @@ TEST_F(RateLimitingTest, SchedulerFailover)
> const string& messages_received =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_received";
> EXPECT_EQ(1u, metrics.values.count(messages_received));
> - EXPECT_EQ(1,
> metrics.values[messages_received].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
>
> const string& messages_processed =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_processed";
> EXPECT_EQ(1u, metrics.values.count(messages_processed));
> - EXPECT_EQ(1,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
> }
>
> // 2. Now launch the second (i.e., failover) scheduler using the
> @@ -898,12 +920,16 @@ TEST_F(RateLimitingTest, SchedulerFailover)
> const string& messages_received =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_received";
> EXPECT_EQ(1u, metrics.values.count(messages_received));
> - EXPECT_EQ(2,
> metrics.values[messages_received].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 2,
> +
> metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
>
> const string& messages_processed =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_processed";
> EXPECT_EQ(1u, metrics.values.count(messages_processed));
> - EXPECT_EQ(1,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
> }
>
> // Need another half a second to have it processed.
> @@ -922,12 +948,16 @@ TEST_F(RateLimitingTest, SchedulerFailover)
> const string& messages_received =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_received";
> EXPECT_EQ(1u, metrics.values.count(messages_received));
> - EXPECT_EQ(2,
> metrics.values[messages_received].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 2,
> +
> metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
>
> const string& messages_processed =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_processed";
> EXPECT_EQ(1u, metrics.values.count(messages_processed));
> - EXPECT_EQ(2,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 2,
> +
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
> }
>
> EXPECT_EQ(DRIVER_STOPPED, driver2.stop());
> @@ -1012,12 +1042,16 @@ TEST_F(RateLimitingTest, CapacityReached)
> const string& messages_received =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_received";
> EXPECT_EQ(1u, metrics.values.count(messages_received));
> - EXPECT_EQ(1,
> metrics.values[messages_received].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
>
> const string& messages_processed =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_processed";
> EXPECT_EQ(1u, metrics.values.count(messages_processed));
> - EXPECT_EQ(1,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
> }
>
> // The subsequent messages are going to be throttled.
> @@ -1064,12 +1098,16 @@ TEST_F(RateLimitingTest, CapacityReached)
> const string& messages_received =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_received";
> EXPECT_EQ(1u, metrics.values.count(messages_received));
> - EXPECT_EQ(5,
> metrics.values[messages_received].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 5,
> +
> metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
> const string& messages_processed =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_processed";
> EXPECT_EQ(1u, metrics.values.count(messages_processed));
> // Four messages not processed, two in the queue and two dropped.
> - EXPECT_EQ(1,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 1,
> +
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
> }
>
> // Advance three times for the two pending messages and the exited
> @@ -1086,12 +1124,16 @@ TEST_F(RateLimitingTest, CapacityReached)
> const string& messages_received =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_received";
> EXPECT_EQ(1u, metrics.values.count(messages_received));
> - EXPECT_EQ(5,
> metrics.values[messages_received].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 5,
> + metrics.values[messages_received].as<JSON::Number>().as<int64_t>());
> const string& messages_processed =
> "frameworks/" + DEFAULT_CREDENTIAL.principal() +
> "/messages_processed";
> EXPECT_EQ(1u, metrics.values.count(messages_processed));
> // Two messages are dropped.
> - EXPECT_EQ(3,
> metrics.values[messages_processed].as<JSON::Number>().value);
> + EXPECT_EQ(
> + 3,
> +
> metrics.values[messages_processed].as<JSON::Number>().as<int64_t>());
>
> Shutdown();
> }
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/2c277f1c/src/tests/slave_tests.cpp
> ----------------------------------------------------------------------
> diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
> index 447c43c..dbe9b1d 100644
> --- a/src/tests/slave_tests.cpp
> +++ b/src/tests/slave_tests.cpp
> @@ -1080,7 +1080,7 @@ TEST_F(SlaveTest, StateEndpoint)
> ASSERT_TRUE(state.values["start_time"].is<JSON::Number>());
> EXPECT_EQ(
> static_cast<int>(Clock::now().secs()),
> -
> static_cast<int>(state.values["start_time"].as<JSON::Number>().value));
> + state.values["start_time"].as<JSON::Number>().as<int>());
>
> // TODO(bmahler): The slave must register for the 'id'
> // to be non-empty.
>
>
