JSON interface for RPC Client [Issue #14] by attaluris · Pull Request #109 · ucbrise/confluo

attaluris · 2018-11-21T09:13:29Z

No description provided.

AmplabJenkins · 2018-11-21T09:20:24Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/confluo-prb/330/
Test FAILed.

AmplabJenkins · 2018-11-24T11:34:00Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/confluo-prb/331/
Test PASSed.

anuragkh · 2018-11-24T20:25:09Z

libconfluo/test/atomic_multilog_test.h

 TEST_F(AtomicMultilogTest, AppendAndGetJSONRecordTest1) {
  atomic_multilog mlog("my_table", s, "/tmp", storage::IN_MEMORY, archival_mode::OFF, MGMT_POOL);

-  std::string rec1 = "{'a':'false', 'b':'0', 'c':'0', 'd':'0', 'e':'0', 'f':'0.000000', 'g':'0.010000', 'h':'abc'}";


We should also test if this syntax works -- this would be the main use-case, I don't expect users to convert it to JSON and then back to string as the test now does.

anuragkh · 2018-11-24T20:26:15Z

libconfluo/confluo/atomic_multilog.h

+   * @param json_data The json-formatted data to be stored
+   * @return The offset of where the data is located
+   */
+  size_t append_json(std::string json_data);


Suggested change

size_t append_json(std::string json_data);

size_t append_json(const std::string &json_data);

anuragkh · 2018-11-24T20:26:32Z

libconfluo/confluo/schema/schema.h

+   *
+   * @return A pointer to the record data
+   */
+  void *json_string_to_data(const std::string json_record) const;


Suggested change

void *json_string_to_data(const std::string json_record) const;

void *json_string_to_data(const std::string &json_record) const;

anuragkh · 2018-11-24T20:27:09Z

libconfluo/confluo/schema/schema.h

+   *
+   * @param record The records used for conversion
+   *
+   * @return A pointer to the record data


This does not return anything..

anuragkh · 2018-11-24T20:27:39Z

libconfluo/src/atomic_multilog.cc

  return offset;
 }

+size_t atomic_multilog::append_json(std::string json_record) {


Suggested change

size_t atomic_multilog::append_json(std::string json_record) {

size_t atomic_multilog::append_json(const std::string &json_record) {

anuragkh · 2018-11-24T20:28:07Z

libconfluo/src/schema/schema.cc

  return str;
 }

+void *schema_t::json_string_to_data(const std::string json_record) const {


Suggested change

void *schema_t::json_string_to_data(const std::string json_record) const {

void *schema_t::json_string_to_data(const std::string &json_record) const {

anuragkh · 2018-11-24T20:29:48Z

libconfluo/test/atomic_multilog_test.h

+
+    std::stringstream ss;
+    pt::write_json(ss, root);
+    std::string ret = ss.str();


Maybe try printing this out and use the printed string as your test input? Tests are also useful as sample usage for users.

anuragkh · 2018-11-24T20:31:35Z

librpc/src/rpc_server.cc

+void rpc_service_handler::read_json(std::string &_return, int64_t id, const int64_t offset, const int64_t nrecords) {
+  atomic_multilog *mlog = store_->get_atomic_multilog(id);
+  _return = mlog->read_json((uint64_t) offset);
+  // TODO: put in functionality for nrecords to be read


Throw unsupported exception instead of incorrect functionality...

anuragkh · 2018-11-24T20:32:18Z

librpc/test/client_read_ops_test.h

+
+    std::stringstream ss;
+    pt::write_json(ss, root);
+    std::string ret = ss.str();


Same thing here -- need to figure out what correct JSON string format is. We need to document it somewhere too.

anuragkh · 2018-11-24T20:32:37Z

librpc/test/client_write_ops_test.h

+
+    std::stringstream ss;
+    pt::write_json(ss, root);
+    std::string ret = ss.str();


Same as before.

anuragkh · 2018-11-24T20:32:58Z

pyclient/confluo/rpc/rpc_service-remote

@@ -0,0 +1,299 @@
+#!/usr/bin/env python


This file should not be checked in.

running genall.sh resulted in this; were there supposed to be no changes?

Yes I know it generates this file, but we don't want to check it in. Ideally the genall.sh file should delete it explicitly -- I can add that change, but can you add a commit to remove the file?

sounds good; are there any other files that shouldn't be checked in?

No, that's the only one.

AmplabJenkins · 2018-12-06T19:00:11Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/confluo-prb/332/
Test PASSed.

anuragkh · 2018-12-06T19:50:43Z

libconfluo/confluo/container/cursor/json_cursors.h

+/**
+ * A json cursor that make records into a json formatted string
+ */
+class aggregated_json_cursor : public json_cursor {


Don't need any aggregation

anuragkh · 2018-12-06T19:52:22Z

libconfluo/confluo/container/cursor/json_cursors.h

+   * @param cexpr The filter expression
+   * @param batch_size The number of records in the batch
+   */
+  aggregated_json_cursor(std::unique_ptr<offset_cursor> o_cursor,


This should take std::unique_ptr<record_cursor> as first argument, const schema_t *as second argument (for converting record to json) and size_t batch_size as the third and last argument.

anuragkh · 2018-12-06T19:53:08Z

libconfluo/src/container/cursor/json_cursors.cc

+  size_t i = 0;
+  for (; i < current_batch_.size() && o_cursor_->has_more();
+         ++i, o_cursor_->advance()) {
+    uint64_t o = o_cursor_->get();


None of this logic will be needed; only need to convert each record in record_cursor to JSON.

AmplabJenkins · 2018-12-14T17:21:53Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/confluo-prb/333/
Test FAILed.

AmplabJenkins · 2018-12-14T18:02:00Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/confluo-prb/334/
Test FAILed.

AmplabJenkins · 2018-12-14T18:09:26Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/confluo-prb/335/
Test FAILed.

…JSON inputs

AmplabJenkins · 2019-01-04T19:11:07Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/confluo-prb/337/
Test FAILed.

JSON interface for RPC client

d6343d2

fixed AppendAndGetJSONRecordTest1

6e0c2a0

anuragkh reviewed Nov 24, 2018

View reviewed changes

anuragkh requested changes Nov 24, 2018

View reviewed changes

anuragkh reviewed Dec 6, 2018

View reviewed changes

implemented json_string_cursor and corresponding multilog filters

0cce8a8

attaluris force-pushed the json-parser branch from fc7ecd1 to 0cce8a8 Compare December 14, 2018 17:18

AppendAndGetJSONRecordTest1 // used printed string as input

af5bdf6

json client_read and write_ops_test // used printed string as input

1e9838c

JSON parser // reduced workload for json_cursor + tested spacing for …

babd20f

…JSON inputs

	size_t append_json(std::string json_data);
	size_t append_json(const std::string &json_data);

	void *json_string_to_data(const std::string json_record) const;
	void *json_string_to_data(const std::string &json_record) const;

	size_t atomic_multilog::append_json(std::string json_record) {
	size_t atomic_multilog::append_json(const std::string &json_record) {

	void *schema_t::json_string_to_data(const std::string json_record) const {
	void *schema_t::json_string_to_data(const std::string &json_record) const {

Conversation

attaluris commented Nov 21, 2018

Uh oh!

AmplabJenkins commented Nov 21, 2018

Uh oh!

AmplabJenkins commented Nov 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Dec 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Dec 14, 2018

Uh oh!

AmplabJenkins commented Dec 14, 2018

Uh oh!

AmplabJenkins commented Dec 14, 2018

Uh oh!

AmplabJenkins commented Jan 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants