Re: [kubernetes/kubernetes] Capture spew.Sprintf() with all our favorite config into a util func (#8976)

[Michail Kargakis]

@kubernetes/sig-api-machinery-misc

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[Tim Hockin]

Another that might be worth considering is https://github.com/sanity-io/litter - I like that emits Go-style literals. Unclear if that can be compacted into 1 line, if it handles map keys and things as well as spew does.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[Tim Hockin]

/lifecycle frozen
/remove-lifecycle stale

[mdhedley]

It doesn't look like anyone has looked at this recently. Would anyone mind if I took a run at wrapping the suggested sanity-io/litter with the suggested util.PrettyPrintObject(interface{}) as suggested? I'm pretty new to go so this seemed like something I could tackle.

[Tim Hockin]

I like litter - give it a shot

On Tue, May 1, 2018 at 8:22 AM mdhedley <notifi...@github.com> wrote:

> It doesn't look like anyone has looked at this recently. Would anyone mind
> if I took a run at wrapping the suggested sanity-io/litter with the
> suggested util.PrettyPrintObject(interface{}) as suggested? I'm pretty new
> to go so this seemed like something I could tackle.
>
> —

> You are receiving this because you authored the thread.

> Reply to this email directly, view it on GitHub

> <https://github.com/kubernetes/kubernetes/issues/8976#issuecomment-385698227>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AFVgVNtMb7Lg1mjN6f-p3lo1hN4Ofdu7ks5tuH20gaJpZM4EuVBn>

[k8s-ci-robot]

@nikhita:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

I'm going to label this a good-first-issue because the changes are relatively easier and there is already a PR (which was closed due to age) that can be used as a reference: #55383

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[Nikhita Raghunath]

I'm going to label this a good-first-issue because the changes are relatively easier and there is already a PR (which was closed due to age) that can be used as a reference: #55383

/good-first-issue

—

[Nikhita Raghunath]

/kind cleanup

[Gokul T P]

I would like to take this up.
/assign

[k8s-ci-robot]

@gokultp: GitHub didn't allow me to assign the following users: gokultp.

Note that only kubernetes members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

I would like to take this up.
/assign

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

—

[Gokul T P]

I compared the performances of both spew and litter packages. attaching the benchmarks below.

goos: linux
goarch: amd64
pkg: github.com/gokultp/bookmark_pretty_printers
BenchmarkLitter-4   	  300000	      4679 ns/op	    1824 B/op	      79 allocs/op
BenchmarkSpew-4   	  500000	      2668 ns/op	     656 B/op	      26 allocs/op
PASS

clearly spew is performing better than litter, so I am using spew for writing the util function.

[Tim Hockin]

This is good insight, but raw perf should not be the only metric. We
should evaluate output for human readability.

On Sun, Sep 30, 2018 at 8:40 PM Gokul T P <notifi...@github.com> wrote:

> I compared the performances of both spew and litter packages. attaching
> the benchmarks below.
>
> goos: linux
> goarch: amd64
> pkg: github.com/gokultp/bookmark_pretty_printers

> BenchmarkLitter-4 <http://github.com/gokultp/bookmark_pretty_printersBenchmarkLitter-4> 300000 4679 ns/op 1824 B/op 79 allocs/op

> BenchmarkSpew-4 500000 2668 ns/op 656 B/op 26 allocs/op
> PASS
>
> clearly spew is performing better than litter, so I am using spew for
> writing the util function.
>
> —

> You are receiving this because you authored the thread.

> Reply to this email directly, view it on GitHub

> <https://github.com/kubernetes/kubernetes/issues/8976#issuecomment-425781589>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AFVgVEcdfw5ZqxyahOVkE8C3esC7Sznbks5ugY65gaJpZM4EuVBn>

[Tim Hockin]

Also there's https://github.com/kr/pretty

Also we should file a bug againstlitter to see if they can do better (I
like their output better than spew)

[Gokul T P]

Actually https://github.com/kr/pretty is working well. here is the benchmarks

BenchmarkPretty-4   	  500000	      2653 ns/op	     360 B/op	      14 allocs/op
BenchmarkLitter-4   	  300000	      4717 ns/op	    1824 B/op	      79 allocs/op
BenchmarkSpew-4   	  500000	      2639 ns/op	     656 B/op	      26 allocs/op

also attaching the formatted outputs

1. spew:

{{foo false 12344} 1234.123 [foo bar]}

2. linter:

main.TestType{
          Field1: main.SubType{
            Field2: "foo",
            Field3: false,
            Field4: 12344,
          },
          Field5: 1234.123,
          Field6: []string{
            "foo",
            "bar",
          },
        }

3. pretty :

main.TestType{Field1:main.SubType{Field2:"foo", Field3:false, Field4:12344}, Field5:1234.123, Field6:[]string{"foo", "bar"}}

so I'll be changing the util function to use https://github.com/kr/pretty, as it is giving better performance and almost the same output.

[Han Kang]

I agree that it may not make a lot of sense to actually migrate anything over here. I personally prefer the standard lib output.

[Gaurav Walecha]

It seems like fanzhangio has already resolved the aim of this ticket. What all things are left in this to work ?

[Akshesh Doshi]

Hi
Can this ticket be marked as closed or is there anything left? I would like to help, if required.

[Tim Garrity]

/close

—
You are receiving this because you are on a team that was mentioned.

Reply to this email directly, view it on GitHub, or unsubscribe.

[Kubernetes Prow Robot]

@Salamandastron1: You can't close an active issue/PR unless you authored it or you are a collaborator.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

—

[Jonathan Elroy Alvares]

/assign

[Jonathan Elroy Alvares]

/unassign

[Madhvi Patel]

Hi,
Is this ticket closed or still open.
I wanted to take this .

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.

Triage notifications on the go with GitHub Mobile for iOS or Android.

[shadower]

/assign

[Yorman]

/close

[Kubernetes Prow Robot]

@cixtor: You can't close an active issue/PR unless you authored it or you are a collaborator.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

—

[Leo Sorkin]

Hi
It looks like this issue now has a separate PR. @thockin could you take a look and see if this is something still viable after 6 years? kubernetes/utils#226

—

Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are on a team that was mentioned.Message ID: <kubernetes/kubernetes/issues/8976/1036524071@github.com>

[Hao Ruan]

Based on kubernetes/utils#226 (comment), we decided to work on k/k first, then move to kubernetes/utils @thockin

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are on a team that was mentioned.Message ID: <kubernetes/kubernetes/issues/8976/1281781983@github.com>

[Kubernetes Prow Robot]

Closed #8976 as completed via #114800.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are on a team that was mentioned.Message ID: <kubernetes/kubernetes/issue/8976/issue_event/8978692564@github.com>