)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"8253e4fe2650f4ccad76d9b07c44bc4e50d2c001","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"26688de8_40b22101","updated":"2021-07-20 11:05:11.000000000","message":"LGTM","commit_id":"f1a34fa8d2261745ec97a00f47323e66228b637d"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"f24b16ae4f8371154d12bdf8b16240e0c1b2dc10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"3ef58b9b_26f17db9","updated":"2021-07-19 16:26:30.000000000","message":"Looks good, I\u0027m waiting with a +1 until I get some clarification regarding the intent/status tracking for the roleserver on CL189.","commit_id":"f1a34fa8d2261745ec97a00f47323e66228b637d"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"355b227e0686e5d188a2ca16ca227658da283881","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"bef32e2a_b510a549","updated":"2021-07-19 09:23:22.000000000","message":"Ping.","commit_id":"f1a34fa8d2261745ec97a00f47323e66228b637d"}],"metropolis/node/core/roleserve/kubernetes_worker.go":[{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"a550185465e43afab4cbcdbc9f6eaf0ae17984c3","unresolved":true,"context_lines":[{"line_number":50,"context_line":"\t\t\tRoot:    s.StorageRoot,"},{"line_number":51,"context_line":"\t\t\tNetwork: s.Network,"},{"line_number":52,"context_line":"\t\t})"},{"line_number":53,"context_line":"\t\tif err :\u003d supervisor.Run(ctx, \"run\", kubeSvc.Run); err !\u003d nil {"},{"line_number":54,"context_line":"\t\t\treturn fmt.Errorf(\"failed to start kubernetes service: %w\", err)"},{"line_number":55,"context_line":"\t\t}"},{"line_number":56,"context_line":"\t\ts.kwSvcC \u003c- kubeSvc"}],"source_content_type":"text/x-go","patch_set":6,"id":"365d665b_5d376184","line":53,"range":{"start_line":53,"start_character":12,"end_line":53,"end_character":26},"updated":"2021-07-07 16:22:57.000000000","message":"This generates very long runnable paths. Just this CL will result in roleserver.kubernetes-worker.run.\n\nIn combination with the already-long path names in the kubernetes package this will result in some unwieldy path names. Is this a good tradeoff vs having less runnables?","commit_id":"ec686f359f31a725e285b5cadc65e9ba7f1ff3c6"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"69aaf9c9591cc3c33415b3777715114ae8484253","unresolved":true,"context_lines":[{"line_number":50,"context_line":"\t\t\tRoot:    s.StorageRoot,"},{"line_number":51,"context_line":"\t\t\tNetwork: s.Network,"},{"line_number":52,"context_line":"\t\t})"},{"line_number":53,"context_line":"\t\tif err :\u003d supervisor.Run(ctx, \"run\", kubeSvc.Run); err !\u003d nil {"},{"line_number":54,"context_line":"\t\t\treturn fmt.Errorf(\"failed to start kubernetes service: %w\", err)"},{"line_number":55,"context_line":"\t\t}"},{"line_number":56,"context_line":"\t\ts.kwSvcC \u003c- kubeSvc"}],"source_content_type":"text/x-go","patch_set":6,"id":"857efe2f_7f53a0c2","line":53,"range":{"start_line":53,"start_character":12,"end_line":53,"end_character":26},"in_reply_to":"365d665b_5d376184","updated":"2021-07-13 18:46:18.000000000","message":"Yeah, I also noticed this.\n\nThere\u0027s a few solutions to this that I can think of:\n\n 1. Limit our runnable name lengths, or rearrange the supervision tree to make the DNs shorter.\n 2. Introduce separate \u0027short\u0027 names for logging and other ops interactions.\n 3. Make the client-side tooling handle these names better, ie. shorten them where possible and requested.\n\nI don\u0027t like 1. as it breaks abstractions: it influences the way we set up runnables because we end up with long DNs that are annoying to interact with when consuming logs. It artificially limits the usefulness of the supervisor because of something somewhat unrelated.\n\nI don\u0027t like 2. as it breaks the symmetry between the supervision tree and the logging DNs. I could be convinced to maybe set up an obviously separate namespace for short log paths (eg. rename root.* to sup[ervisor].*, and introduce a shorter named.* where runnable could alias themselves into when requested). But that\u0027s a large design change, and there\u0027s a bunch of corner casese to consider.\n\n3. seems to me the least impedance-mismsatched and fastest to implement - there\u0027s a couple of low hanging fruits we could easily implement, like some name shortening scheme for output stdout logs (eg. rroot.roleserver.kubernetes-worker.run.controller-manager becomes r.r.kw.r.controller-manager), tab completion in the dbg client, ... And if that doesn\u0027t work, we can maybe think again of introducing 2.? But I really wouldn\u0027t like to modify anything related to supervision DNs just to make them \u0027easier\u0027 to use by operators, they should IMO continue being as descriptive as possible.","commit_id":"ec686f359f31a725e285b5cadc65e9ba7f1ff3c6"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"f24b16ae4f8371154d12bdf8b16240e0c1b2dc10","unresolved":false,"context_lines":[{"line_number":50,"context_line":"\t\t\tRoot:    s.StorageRoot,"},{"line_number":51,"context_line":"\t\t\tNetwork: s.Network,"},{"line_number":52,"context_line":"\t\t})"},{"line_number":53,"context_line":"\t\tif err :\u003d supervisor.Run(ctx, \"run\", kubeSvc.Run); err !\u003d nil {"},{"line_number":54,"context_line":"\t\t\treturn fmt.Errorf(\"failed to start kubernetes service: %w\", err)"},{"line_number":55,"context_line":"\t\t}"},{"line_number":56,"context_line":"\t\ts.kwSvcC \u003c- kubeSvc"}],"source_content_type":"text/x-go","patch_set":6,"id":"9dfbe65b_ca113378","line":53,"range":{"start_line":53,"start_character":12,"end_line":53,"end_character":26},"in_reply_to":"857efe2f_7f53a0c2","updated":"2021-07-19 16:26:30.000000000","message":"Yeah, it\u0027s a hard problem.\n\nI definitely agree with 3. because it doesn\u0027t require any changes on the supervisor side but in certain cases (mainly with launch-only tree nodes) it might make sense to just straight-up shorten the path (for example by introducing a way to replace the current supervisor service with the launched one).\n\nI filed https://github.com/monogon-dev/monogon/issues/71 to track this.","commit_id":"ec686f359f31a725e285b5cadc65e9ba7f1ff3c6"}],"metropolis/node/core/roleserve/roleserve.go":[{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"a550185465e43afab4cbcdbc9f6eaf0ae17984c3","unresolved":true,"context_lines":[{"line_number":138,"context_line":"// library, maybe one that implements the Event Value interface."},{"line_number":139,"context_line":"func (s *Service) runUpdater(ctx context.Context) error {"},{"line_number":140,"context_line":"\tsupervisor.Logger(ctx).Info(\"Dialing curator...\")"},{"line_number":141,"context_line":"\tcluster, err :\u003d s.CuratorDial(ctx)"},{"line_number":142,"context_line":"\tif err !\u003d nil {"},{"line_number":143,"context_line":"\t\treturn fmt.Errorf(\"could not dial cluster curator: %w\", err)"},{"line_number":144,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":9,"id":"b3fb19b4_bef15a68","line":141,"updated":"2021-07-07 16:22:57.000000000","message":"Why are we redialing the curator every time the watch fails? gRPC clients have an internal connection pool and thus redialing should be pointless, right?","commit_id":"3306f68939d4d37b11352ad398a685752e79bf4d"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"69aaf9c9591cc3c33415b3777715114ae8484253","unresolved":false,"context_lines":[{"line_number":138,"context_line":"// library, maybe one that implements the Event Value interface."},{"line_number":139,"context_line":"func (s *Service) runUpdater(ctx context.Context) error {"},{"line_number":140,"context_line":"\tsupervisor.Logger(ctx).Info(\"Dialing curator...\")"},{"line_number":141,"context_line":"\tcluster, err :\u003d s.CuratorDial(ctx)"},{"line_number":142,"context_line":"\tif err !\u003d nil {"},{"line_number":143,"context_line":"\t\treturn fmt.Errorf(\"could not dial cluster curator: %w\", err)"},{"line_number":144,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":9,"id":"9545bb21_05c6e0bc","line":141,"in_reply_to":"b3fb19b4_bef15a68","updated":"2021-07-13 18:46:18.000000000","message":"Done","commit_id":"3306f68939d4d37b11352ad398a685752e79bf4d"}]}