Adding glustercli command to get options of all volumes #1406

rishubhjain · 2018-12-14T01:07:40Z

Fixes:#1385
Signed-off-by: rishubhjain rishubhjain47@gmail.com

rishubhjain · 2018-12-14T01:08:23Z

@Madhu-1 @atinmu Please review

Madhu-1 · 2018-12-14T03:10:28Z

retest this please

Madhu-1

please paste the sample output for below scenario.

volume option for one volume
options for all

rishubhjain · 2018-12-14T03:50:55Z

Volume option for all volumes :

[root@localhost glusterd2]# ./build/glustercli volume get all all
Volume : testvol1
+-------------------------------+----------+--------+---------------+--------------+
|             NAME              | MODIFIED | VALUE  | DEFAULT VALUE | OPTION LEVEL |
+-------------------------------+----------+--------+---------------+--------------+
| nufa.lookup-unhashed          | no       | on     | on            | Basic        |
| nufa.min-free-disk            | no       | 10%    | 10%           | Basic        |
| nufa.min-free-inodes          | no       | 5%     | 5%            | Basic        |
| nufa.rebalance-stats          | no       | off    | off           | Basic        |
| nufa.rsync-hash-regex         | no       |        |               | Basic        |
| nufa.extra-hash-regex         | no       |        |               | Basic        |
| nufa.weighted-rebalance       | no       | on     | on            | Basic        |
| nufa.rebal-throttle           | no       | normal | normal        | Basic        |
| distribute.lookup-unhashed    | no       | on     | on            | Basic        |
| distribute.min-free-disk      | no       | 10%    | 10%           | Basic        |
| distribute.min-free-inodes    | no       | 5%     | 5%            | Basic        |
| distribute.rebalance-stats    | no       | off    | off           | Basic        |
| distribute.rsync-hash-regex   | no       |        |               | Basic        |
| distribute.extra-hash-regex   | no       |        |               | Basic        |
| distribute.weighted-rebalance | no       | on     | on            | Basic        |
| distribute.rebal-throttle     | no       | normal | normal        | Basic        |
| changelog.changelog           | no       | off    | off           | Basic        |
| changelog.capture-del-path    | no       | off    | off           | Basic        |
| switch.lookup-unhashed        | no       | on     | on            | Basic        |
| switch.min-free-disk          | no       | 10%    | 10%           | Basic        |
| switch.min-free-inodes        | no       | 5%     | 5%            | Basic        |
| switch.rebalance-stats        | no       | off    | off           | Basic        |
| switch.rsync-hash-regex       | no       |        |               | Basic        |
| switch.extra-hash-regex       | no       |        |               | Basic        |
| switch.weighted-rebalance     | no       | on     | on            | Basic        |
| switch.rebal-throttle         | no       | normal | normal        | Basic        |
| quota.enable                  | no       | off    | off           | Basic        |
+-------------------------------+----------+--------+---------------+--------------+
Volume : testvol2
+-------------------------------+----------+--------+---------------+--------------+
|             NAME              | MODIFIED | VALUE  | DEFAULT VALUE | OPTION LEVEL |
+-------------------------------+----------+--------+---------------+--------------+
| quota.enable                  | no       | off    | off           | Basic        |
| nufa.lookup-unhashed          | no       | on     | on            | Basic        |
| nufa.min-free-disk            | no       | 10%    | 10%           | Basic        |
| nufa.min-free-inodes          | no       | 5%     | 5%            | Basic        |
| nufa.rebalance-stats          | no       | off    | off           | Basic        |
| nufa.rsync-hash-regex         | no       |        |               | Basic        |
| nufa.extra-hash-regex         | no       |        |               | Basic        |
| nufa.weighted-rebalance       | no       | on     | on            | Basic        |
| nufa.rebal-throttle           | no       | normal | normal        | Basic        |
| distribute.lookup-unhashed    | no       | on     | on            | Basic        |
| distribute.min-free-disk      | no       | 10%    | 10%           | Basic        |
| distribute.min-free-inodes    | no       | 5%     | 5%            | Basic        |
| distribute.rebalance-stats    | no       | off    | off           | Basic        |
| distribute.rsync-hash-regex   | no       |        |               | Basic        |
| distribute.extra-hash-regex   | no       |        |               | Basic        |
| distribute.weighted-rebalance | no       | on     | on            | Basic        |
| distribute.rebal-throttle     | no       | normal | normal        | Basic        |
| changelog.changelog           | no       | off    | off           | Basic        |
| changelog.capture-del-path    | no       | off    | off           | Basic        |
| switch.lookup-unhashed        | no       | on     | on            | Basic        |
| switch.min-free-disk          | no       | 10%    | 10%           | Basic        |
| switch.min-free-inodes        | no       | 5%     | 5%            | Basic        |
| switch.rebalance-stats        | no       | off    | off           | Basic        |
| switch.rsync-hash-regex       | no       |        |               | Basic        |
| switch.extra-hash-regex       | no       |        |               | Basic        |
| switch.weighted-rebalance     | no       | on     | on            | Basic        |
| switch.rebal-throttle         | no       | normal | normal        | Basic        |
+-------------------------------+----------+--------+---------------+--------------+

Volume option for one volume:

[root@localhost glusterd2]# ./build/glustercli volume get testvol1 all
Volume : testvol1
+-------------------------------+----------+--------+---------------+--------------+
|             NAME              | MODIFIED | VALUE  | DEFAULT VALUE | OPTION LEVEL |
+-------------------------------+----------+--------+---------------+--------------+
| changelog.changelog           | no       | off    | off           | Basic        |
| changelog.capture-del-path    | no       | off    | off           | Basic        |
| switch.lookup-unhashed        | no       | on     | on            | Basic        |
| switch.min-free-disk          | no       | 10%    | 10%           | Basic        |
| switch.min-free-inodes        | no       | 5%     | 5%            | Basic        |
| switch.rebalance-stats        | no       | off    | off           | Basic        |
| switch.rsync-hash-regex       | no       |        |               | Basic        |
| switch.extra-hash-regex       | no       |        |               | Basic        |
| switch.weighted-rebalance     | no       | on     | on            | Basic        |
| switch.rebal-throttle         | no       | normal | normal        | Basic        |
| quota.enable                  | no       | off    | off           | Basic        |
| nufa.lookup-unhashed          | no       | on     | on            | Basic        |
| nufa.min-free-disk            | no       | 10%    | 10%           | Basic        |
| nufa.min-free-inodes          | no       | 5%     | 5%            | Basic        |
| nufa.rebalance-stats          | no       | off    | off           | Basic        |
| nufa.rsync-hash-regex         | no       |        |               | Basic        |
| nufa.extra-hash-regex         | no       |        |               | Basic        |
| nufa.weighted-rebalance       | no       | on     | on            | Basic        |
| nufa.rebal-throttle           | no       | normal | normal        | Basic        |
| distribute.lookup-unhashed    | no       | on     | on            | Basic        |
| distribute.min-free-disk      | no       | 10%    | 10%           | Basic        |
| distribute.min-free-inodes    | no       | 5%     | 5%            | Basic        |
| distribute.rebalance-stats    | no       | off    | off           | Basic        |
| distribute.rsync-hash-regex   | no       |        |               | Basic        |
| distribute.extra-hash-regex   | no       |        |               | Basic        |
| distribute.weighted-rebalance | no       | on     | on            | Basic        |
| distribute.rebal-throttle     | no       | normal | normal        | Basic        |
+-------------------------------+----------+--------+---------------+--------------+

rishubhjain · 2018-12-14T03:51:45Z

retest this please

Madhu-1 · 2018-12-14T03:56:14Z

[root@localhost +------------------------ | NAME +------------------------ | changelog.changelog | changelog.capture-del-path | quota.enable | distribute.lookup-unhashed | distribute.min-free-disk | distribute.min-free-inodes | distribute.rebalance-stats | distribute.rsync-hash-regex | distribute.extra-hash-regex | distribute.weighted-rebalance | no | distribute.rebal-throttle | nufa.lookup-unhashed | nufa.min-free-disk | nufa.min-free-inodes | nufa.rebalance-stats | nufa.rsync-hash-regex | nufa.extra-hash-regex | nufa.weighted-rebalance | nufa.rebal-throttle | switch.lookup-unhashed | switch.min-free-disk | switch.min-free-inodes | switch.rebalance-stats | switch.rsync-hash-regex | switch.extra-hash-regex | switch.weighted-rebalance | switch.rebal-throttle +------------------------ glusterd2]# ./build/glustercli volume get testvol1 all
-------+----------+--------+---------------+--------------+
| MODIFIED | VALUE | DEFAULT VALUE | OPTION LEVEL |
-------+----------+--------+---------------+--------------+
| no | off | off | Basic |
| no | off | off | Basic |
| no | off | off | Basic |
| no | on | on | Basic |
| no | 10% | 10% | Basic |
| no | 5% | 5% | Basic |
| no | off | off | Basic |
| no | | | Basic |
| no | | | Basic |
| on | on | Basic |
| no | normal | normal | Basic |
| no | on | on | Basic |
| no | 10% | 10% | Basic |
| no | 5% | 5% | Basic |
| no | off | off | Basic |
| no | | | Basic |
| no | | | Basic |
| no | on | on | Basic |
| no | normal | normal | Basic |
| no | on | on | Basic |
| no | 10% | 10% | Basic |
| no | 5% | 5% | Basic |
| no | off | off | Basic |
| no | | | Basic |
| no | | | Basic |
| no | on | on | Basic |
| no | normal | normal | Basic |
-------+----------+--------+---------------+--------------+

am i missing anything, the table you declared as the volume name field, but its missing here?

Madhu-1 · 2018-12-14T03:59:02Z

glustercli/cmd/volume.go

-			log.WithError(err).Error("error getting volume options")
-			failure("Error getting volume options", err, 1)
+		var volList []string
+		if volname == "all" {


what happens if the user creates a volume with the name all and want to fetch the volume option for that particular volume :D.

good point, we need to reserve these keywords(all, status, volume etc) and not allow volume create.

Nice Catch, should I add this to volume-create?

aravindavk · 2018-12-14T04:27:17Z

For uniformity you can retain "Volume name" even in single volume output. Or follow convention similar to volume status

volume: <name>
TABLE

volume: <name>
TABLE

rishubhjain · 2018-12-14T13:27:47Z

@aravindavk Made the changes, can you check if this is what you wanted?
@Madhu-1 @aravindavk The new output is #1406 (comment)

aravindavk · 2018-12-14T15:03:13Z

glustercli/cmd/volume.go


+				}
+			}
+			table.Render()


print one extra line after the table

aravindavk · 2018-12-14T15:07:05Z

glusterd2/commands/volumes/volume-create.go

@@ -42,6 +43,11 @@ func validateVolCreateReq(req *api.VolCreateReq) error {
 		return gderrors.ErrInvalidVolName
 	}

+	if gutils.IsReservedKeyword(req.Name) {
+		errMsg := fmt.Sprintf("invalid name, volume name cannot be among GD2 reserved keywords:%v", gutils.ReservedKeywords)
+		return errors.New(errMsg)


you can use fmt.Errorf

return fmt.Errorf("invalid name, volume name cannot be among GD2 reserved keywords:%v", gutils.ReservedKeywords)

aravindavk · 2018-12-14T15:07:56Z

pkg/utils/utils.go

@@ -31,6 +31,17 @@ const (
 	TiB = 1024 * GiB
 )

+var (
+	// ReservedKeywords are Glusterd2 reserved keywords
+	ReservedKeywords = []string{"all", "volume", "status", "list"}


also include "cluster", "gluster", "brick"

aravindavk · 2018-12-14T15:13:03Z

Issue #1385 talks about showing cluster options. But this PR only includes showing options of all volumes. Cluster options are available in https://github.com/gluster/glusterd2/blob/master/glusterd2/commands/options/commands.go#L25 and the list of valid options are maintained here https://github.com/gluster/glusterd2/blob/master/glusterd2/options/cluster.go#L29

rishubhjain · 2018-12-15T14:12:54Z

[root@localhost glusterd2]# ./build/glustercli cluster get
+--------------------------------+----------+-------+---------------+
|              NAME              | MODIFIED | VALUE | DEFAULT VALUE |
+--------------------------------+----------+-------+---------------+
| cluster.brick-multiplex        | no       | off   | off           |
| cluster.max-bricks-per-process | no       | 0     | 0             |
| cluster.localtime-logging      | no       | off   | off           |
| cluster.shared-storage         | no       | off   | off           |
| cluster.op-version             | no       | 50000 | 50000         |
| cluster.max-op-version         | no       | 50000 | 50000         |
+--------------------------------+----------+-------+---------------+
[root@localhost glusterd2]# ./build/glustercli cluster set cluster.brick-multiplex 1000
Options set successfully 
[root@localhost glusterd2]# ./build/glustercli cluster get
+--------------------------------+----------+-------+---------------+
|              NAME              | MODIFIED | VALUE | DEFAULT VALUE |
+--------------------------------+----------+-------+---------------+
| cluster.shared-storage         | no       | off   | off           |
| cluster.op-version             | no       | 50000 | 50000         |
| cluster.max-op-version         | no       | 50000 | 50000         |
| cluster.brick-multiplex        | yes      | 1000  | off           |
| cluster.max-bricks-per-process | no       | 0     | 0             |
| cluster.localtime-logging      | no       | off   | off           |
+--------------------------------+----------+-------+---------------+

Madhu-1 · 2018-12-15T16:40:01Z

glustercli/cmd/cluster-options.go

+		}
+		failure("Cluster option set failed", err, 1)
+	} else {
+		fmt.Printf("Options set successfully \n")


why can't you use fmt.Println()?

Would it make a big difference?

we already have a function that provides the same functionality, what's the issue in using default function?

Madhu-1 · 2018-12-15T16:40:39Z

glustercli/cmd/cluster-options.go

+
+	err := client.ClusterOptionSet(api.ClusterOptionReq{
+		Options: copt})
+	if err != nil {


return err

I am returning err at line 75, am I missing something?

what is the advantage you are getting by doing the error check here?

Madhu-1 · 2018-12-15T16:41:57Z

glustercli/cmd/cluster-options.go

+var clusterOptionGetCmd = &cobra.Command{
+	Use:   "get",
+	Short: helpClusterOptionGetCmd,
+	Args:  cobra.RangeArgs(0, 1),


where you are making use of args here?

Madhu-1 · 2018-12-15T16:43:22Z

glusterd2/commands/volumes/volume-create.go

@@ -42,6 +43,10 @@ func validateVolCreateReq(req *api.VolCreateReq) error {
 		return gderrors.ErrInvalidVolName
 	}

+	if gutils.IsReservedKeyword(req.Name) {
+		return fmt.Errorf("invalid name, volume name cannot be among glusterd reserved keywords:%v", gutils.ReservedKeywords)


glusterd or glusterd2?

That's a point I want to discuss, will users be always referring the glustercli daemon as gluster daemon 2 or is it just temperory.

@aravindavk ^^

you can skip that word itself. "invalid name, volume name cannot be among reserved keywords"

Madhu-1 · 2018-12-15T16:43:55Z

glusterd2/commands/volumes/volume-create.go

@@ -42,6 +43,10 @@ func validateVolCreateReq(req *api.VolCreateReq) error {
 		return gderrors.ErrInvalidVolName
 	}

+	if gutils.IsReservedKeyword(req.Name) {


please add a e2e test case of this

Same as mentioned for other case, please add this as well in the same issue

these code changes are in this PR so that I would like to see an e2e in this PR itself.

Madhu-1 · 2018-12-15T16:44:11Z

pkg/restclient/cluster.go

+)
+
+// GetClusterOption gets cluster level options
+func (c *Client) GetClusterOption() ([]api.ClusterOptionsResp, error) {


add e2e test case of this.

Not a part of this PR, this PR only addresses glustercli commands, I will add e2e test cases for cluster option in separate PR, please raise an issue for it will fix that issue.

why you would like to add Client library in one PR and add e2e in other PR. what the concern here?
even if you are adding a functionality for CLI, you are adding a client library.

Madhu-1 · 2018-12-15T16:46:20Z

pkg/utils/utils.go

@@ -31,6 +31,17 @@ const (
 	TiB = 1024 * GiB
 )

+var (
+	// ReservedKeywords are Glusterd2 reserved keywords
+	ReservedKeywords = []string{"all", "volume", "status", "list", "cluster", "gluster", "brick"}


add comment for why we are reserving these keywords

Reserved key words is self explanatory

I wanted to see, why we are reserving these keywords, this will help the developers who will join later.
by these comments, I don't think they will get more context why these keywords are reserved.

Madhu-1 · 2018-12-16T04:11:18Z

glustercli/cmd/cluster-options.go

+		}
+		failure("Cluster option set failed", err, 1)
+	} else {
+		fmt.Printf("Options set successfully \n")


we already have a function that provides the same functionality, what's the issue in using default function?

Madhu-1 · 2018-12-16T04:12:22Z

glustercli/cmd/cluster-options.go

+
+	err := client.ClusterOptionSet(api.ClusterOptionReq{
+		Options: copt})
+	if err != nil {


what is the advantage you are getting by doing the error check here?

Madhu-1 · 2018-12-16T04:13:30Z

glusterd2/commands/volumes/volume-create.go

@@ -42,6 +43,10 @@ func validateVolCreateReq(req *api.VolCreateReq) error {
 		return gderrors.ErrInvalidVolName
 	}

+	if gutils.IsReservedKeyword(req.Name) {


these code changes are in this PR so that I would like to see an e2e in this PR itself.

Madhu-1 · 2018-12-16T04:13:47Z

glusterd2/commands/volumes/volume-create.go

@@ -42,6 +43,10 @@ func validateVolCreateReq(req *api.VolCreateReq) error {
 		return gderrors.ErrInvalidVolName
 	}

+	if gutils.IsReservedKeyword(req.Name) {
+		return fmt.Errorf("invalid name, volume name cannot be among glusterd reserved keywords:%v", gutils.ReservedKeywords)


@aravindavk ^^

Madhu-1 · 2018-12-16T04:16:08Z

pkg/restclient/cluster.go

+)
+
+// GetClusterOption gets cluster level options
+func (c *Client) GetClusterOption() ([]api.ClusterOptionsResp, error) {


why you would like to add Client library in one PR and add e2e in other PR. what the concern here?
even if you are adding a functionality for CLI, you are adding a client library.

Madhu-1 · 2018-12-16T04:18:04Z

pkg/utils/utils.go

@@ -31,6 +31,17 @@ const (
 	TiB = 1024 * GiB
 )

+var (
+	// ReservedKeywords are Glusterd2 reserved keywords
+	ReservedKeywords = []string{"all", "volume", "status", "list", "cluster", "gluster", "brick"}


I wanted to see, why we are reserving these keywords, this will help the developers who will join later.
by these comments, I don't think they will get more context why these keywords are reserved.

atinmu · 2018-12-17T04:04:48Z

[root@localhost glusterd2]# ./build/glustercli cluster get
+--------------------------------+----------+-------+---------------+
| NAME | MODIFIED | VALUE | DEFAULT VALUE |
+--------------------------------+----------+-------+---------------+
| cluster.shared-storage | no | off | off |
| cluster.op-version | no | 50000 | 50000 |
| cluster.max-op-version | no | 50000 | 50000 |
| cluster.brick-multiplex | yes | 1000 | off |
| cluster.max-bricks-per-process | no | 0 | 0 |
| cluster.localtime-logging | no | off | off |
+--------------------------------+----------+-------+---------------+

Can we please clean up the global option tables with all the unwanted options which were defined as place holders to begin in. For eg : cluster.localtime-logging , cluster.shared-storage are not applicable. Also cluster.op-version, cluster,max-op-version values need change and it should stay with current glusterfs master.

Also I'm not particularly sure about if the cli naming convention is correct here. glustercli cluster get doesn't give a proper meaning. Would like to hear thoughts from @aravindavk @kshlm on this.

rishubhjain · 2018-12-18T11:22:31Z

@atinmu Regarding cli naming convention in #1406 (comment), I tried to keep it uniform with volume options glustercli volume get <volname>.

Signed-off-by: rishubhjain <rishubhjain47@gmail.com>

atinmu · 2019-01-04T06:29:00Z

@Madhu-1 Are we good with this PR?

Madhu-1

need small changes or else good to go.

Madhu-1 · 2019-01-04T06:35:40Z

e2e/cluster_test.go

+	r.NotNil(clusterOps)
+}
+
+func ClusterOptionsSet(t *testing.T) {


am not able to find out where this is getting called?

Madhu-1 · 2019-01-04T06:38:37Z

glustercli/cmd/cluster-options.go

+			log.WithError(err).Error("cluster option set failed")
+		}
+		failure("Cluster option set failed", err, 1)
+	} else {


we can remove this else block, as failure does exit 1.

Madhu-1 · 2019-01-04T06:44:20Z

glusterd2/options/cluster.go

@@ -27,12 +27,10 @@ type validateFunc func(string, string) error

 // ClusterOptMap contains list of supported cluster-wide options, default values and value types
 var ClusterOptMap = map[string]*ClusterOption{
-	"cluster.shared-storage":         {"cluster.shared-storage", "off", OptionTypeBool, nil},


any specific reason for removing this option?

shared-storage feature isn't a need in GD2 as of now and such feature doesn't exist yet.

Madhu-1 · 2019-01-04T06:46:46Z

glustercli/cmd/volume.go

@@ -233,39 +233,55 @@ var volumeDeleteCmd = &cobra.Command{
 }

 var volumeGetCmd = &cobra.Command{
-	Use:   "get",
+	Use:   "get <VOLNAME|all> <key|all>",


can we rename key to option as we are getting option details?

what ever we chose here (In GD1's CLI it's referred as key though), request it to be used consistently across.

atinmu · 2019-01-07T05:25:55Z

Can we please close down this PR today? It's been hanging around for a while now :-)

aravindavk · 2019-01-29T03:13:08Z

@rishubhjain please address the comments and rebase

rishubhjain · 2019-01-29T06:37:26Z

@aravindavk Is this a GCS 1.0 blocker?

ghost assigned rishubhjain Dec 14, 2018

ghost added the in progress label Dec 14, 2018

rishubhjain force-pushed the cluster-wide-options branch from db798a6 to cd8527b Compare December 14, 2018 01:28

Madhu-1 reviewed Dec 14, 2018

View reviewed changes

rishubhjain force-pushed the cluster-wide-options branch from 4a5645d to 6f0f2ba Compare December 14, 2018 13:30

aravindavk reviewed Dec 14, 2018

View reviewed changes

rishubhjain force-pushed the cluster-wide-options branch from 6f0f2ba to 49f2c8d Compare December 15, 2018 14:09

rishubhjain force-pushed the cluster-wide-options branch from 49f2c8d to 6dde8aa Compare December 15, 2018 14:25

Madhu-1 suggested changes Dec 15, 2018

View reviewed changes

Madhu-1 suggested changes Dec 16, 2018

View reviewed changes

rishubhjain force-pushed the cluster-wide-options branch from 14fc355 to 95ef5aa Compare December 18, 2018 09:14

rishubhjain force-pushed the cluster-wide-options branch from 4ce21c4 to af6ec95 Compare December 18, 2018 16:09

Adding glustercli command for cluster options and volume options

7fbccba

Signed-off-by: rishubhjain <rishubhjain47@gmail.com>

rishubhjain force-pushed the cluster-wide-options branch from af6ec95 to 7fbccba Compare December 18, 2018 16:37

Madhu-1 suggested changes Jan 4, 2019

View reviewed changes

Adding glustercli command to get options of all volumes #1406

Are you sure you want to change the base?

Adding glustercli command to get options of all volumes #1406

Conversation

rishubhjain commented Dec 14, 2018

rishubhjain commented Dec 14, 2018

Madhu-1 commented Dec 14, 2018

Madhu-1 left a comment

Choose a reason for hiding this comment

rishubhjain commented Dec 14, 2018 • edited Loading

rishubhjain commented Dec 14, 2018

Madhu-1 commented Dec 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rishubhjain Dec 14, 2018 • edited Loading

Choose a reason for hiding this comment

aravindavk commented Dec 14, 2018

rishubhjain commented Dec 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aravindavk commented Dec 14, 2018

rishubhjain commented Dec 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atinmu commented Dec 17, 2018

rishubhjain commented Dec 18, 2018

atinmu commented Jan 4, 2019

Madhu-1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atinmu commented Jan 7, 2019

aravindavk commented Jan 29, 2019

rishubhjain commented Jan 29, 2019

rishubhjain commented Dec 14, 2018 •

edited

Loading

rishubhjain Dec 14, 2018 •

edited

Loading

rishubhjain commented Dec 15, 2018 •

edited

Loading