You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/11/21 07:02:24 UTC

[GitHub] [beam] Malarg opened a new pull request, #24283: Example from embedded to standalone playground sent by path

Malarg opened a new pull request, #24283:
URL: https://github.com/apache/beam/pull/24283

   Code from embedded playground to standalone sent by path
   resolves #23252 
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/get-started-contributing/#make-the-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Go tests](https://github.com/apache/beam/workflows/Go%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] alexeyinkin commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1034392658


##########
playground/frontend/playground_components/lib/src/models/example.dart:
##########
@@ -49,8 +53,11 @@ class Example extends ExampleBase {
     required this.logs,
     required this.outputs,
     required this.source,
+    ExampleLoadingDescriptor? descriptor,
     this.graph,
-  }) : super(
+  })  : descriptor =
+            descriptor ?? StandardExampleLoadingDescriptor(path: example.path),

Review Comment:
   We should make no such assumption. `path` here is just a poorly chosen name from the age when we only had standard examples. Now it can be used for anything.
   
   I am thinking of these options:
   - To make the descriptor required.
   - To default to `ContentExampleLoadingDescriptor` which is safest one although the least shareable one.
   



##########
playground/frontend/playground_components/test/src/controllers/snippet_editing_controller_test.dart:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'package:flutter_test/flutter_test.dart';
+import 'package:playground_components/src/controllers/snippet_editing_controller.dart';
+import 'package:playground_components/src/models/sdk.dart';
+
+import '../common/examples.dart';
+
+void main() {
+  group(
+    'Snippet editing controller',
+    () {
+      test(
+        'Returns standard descriptor if code has not been changed',
+        () {
+          final controller = SnippetEditingController(sdk: Sdk.python);
+          controller.selectedExample = exampleMock1;
+
+          final descriptor = controller.getLoadingDescriptor();
+
+          expect(descriptor.toJson(), {'example': 'SDK_PYTHON/Category/Name1'});
+        },
+      );
+
+      test(
+        'Returns content descriptor if code has been changed',
+        () {
+          final controller = SnippetEditingController(sdk: Sdk.python);
+          controller.selectedExample = exampleMock1;
+
+          controller.codeController.value = const TextEditingValue(text: 'ex4');
+          final descriptor = controller.getLoadingDescriptor();
+
+          expect(descriptor.toJson(), {
+            'sdk': 'python',
+            'content': 'ex4',
+            'name': 'Example X1',
+            'complexity': 'basic',
+          });

Review Comment:
   It is sufficient to test that it is `ContentExampleLoadingDescriptor` with all the fields. Otherwise this test has one more reason to change if the fields are renamed in the descriptor.
   
   The descriptor's ability to be recovered in `toJson` -> `tryParse` should be tested separately.



##########
playground/frontend/lib/pages/embedded_playground/components/embedded_actions.dart:
##########
@@ -58,16 +59,24 @@ class EmbeddedActions extends StatelessWidget {
   void _openStandalonePlayground(PlaygroundController controller) {
     // The empty list forces the parsing of EmptyExampleLoadingDescriptor
     // and prevents the glimpse of the default catalog example.
+
+    final descriptor = controller.getLoadingDescriptor();
+    final urlDescriptor = descriptor.where((d) => d.canBePassedByUrl);
+    final jsDescriptor = descriptor.where((d) => !d.canBePassedByUrl);
+
+    final json = jsonEncode(urlDescriptor.toJson()['descriptors']);

Review Comment:
   This way, if `ExamplesLoadingDescriptor` contains anything else but `.descriptors`, it is lost and we have a false sense that it is not. And it does contain `lazyLoadDescriptors` although this is not used in the embedded playground.
   
   Also this way `['descriptors']` is compile-time unsafe. If it is renamed, we sure will forget to update it here.
   
   I suggest working with `descriptor.descriptors` directly. This way:
   1. We do not have the false sense of completeness when passing the entire `jsDescriptor` in the message.
   2. We cut one level of indirection of `ExamplesLoadingDescriptor.where`.
   3. We eliminate the confusion of `ExamplesLoadingDescriptor.where` losing data.
   



##########
playground/frontend/playground_components/lib/src/models/example_loading_descriptors/example_loading_descriptor.dart:
##########
@@ -28,4 +29,6 @@ abstract class ExampleLoadingDescriptor with EquatableMixin {
   final ExampleViewOptions viewOptions;
 
   Map<String, dynamic> toJson() => throw UnimplementedError();
+
+  bool get canBePassedByUrl => this is! ContentExampleLoadingDescriptor;

Review Comment:
   `canBePassedInUrl`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] alexeyinkin commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1033233627


##########
playground/frontend/playground_components/test/src/controllers/snippet_editing_controllers_test.dart:
##########
@@ -0,0 +1,55 @@
+import 'package:flutter_test/flutter_test.dart';

Review Comment:
   Run `./gradlew rat` to spot files without licenses.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] alexeyinkin commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1027713411


##########
playground/frontend/lib/pages/embedded_playground/components/embedded_actions.dart:
##########
@@ -58,16 +57,12 @@ class EmbeddedActions extends StatelessWidget {
   void _openStandalonePlayground(PlaygroundController controller) {
     // The empty list forces the parsing of EmptyExampleLoadingDescriptor
     // and prevents the glimpse of the default catalog example.
-    final window = html.window.open(
-      '/?$kExamplesParam=[]&$kSdkParam=${controller.sdk?.id}',
-      '',
-    );
 
-    javaScriptPostMessageRepeated(
-      window,
-      SetContentMessage(
-        descriptor: controller.getLoadingDescriptor(),
-      ),
+    var descriptors = controller.getLoadingDescriptor().toJson()['descriptors'];
+    var json = jsonEncode(descriptors);
+    html.window.open(
+      '/?$kExamplesParam=$json&$kSdkParam=${controller.sdk?.id}',
+      '',

Review Comment:
   This would exceed the URL length of browser standards.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] Malarg commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
Malarg commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1035670657


##########
playground/frontend/playground_components/lib/src/models/example.dart:
##########
@@ -49,8 +53,11 @@ class Example extends ExampleBase {
     required this.logs,
     required this.outputs,
     required this.source,
+    ExampleLoadingDescriptor? descriptor,
     this.graph,
-  }) : super(
+  })  : descriptor =
+            descriptor ?? StandardExampleLoadingDescriptor(path: example.path),

Review Comment:
   fixed



##########
playground/frontend/playground_components/lib/src/models/example_loading_descriptors/example_loading_descriptor.dart:
##########
@@ -28,4 +29,6 @@ abstract class ExampleLoadingDescriptor with EquatableMixin {
   final ExampleViewOptions viewOptions;
 
   Map<String, dynamic> toJson() => throw UnimplementedError();
+
+  bool get canBePassedByUrl => this is! ContentExampleLoadingDescriptor;

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] alexeyinkin commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1033189284


##########
playground/frontend/lib/pages/embedded_playground/components/embedded_actions.dart:
##########
@@ -58,16 +59,26 @@ class EmbeddedActions extends StatelessWidget {
   void _openStandalonePlayground(PlaygroundController controller) {
     // The empty list forces the parsing of EmptyExampleLoadingDescriptor
     // and prevents the glimpse of the default catalog example.
+
+    final loadingDescriptor = controller.getLoadingDescriptor();
+    final contentDescriptor =
+        loadingDescriptor.whereType<ContentExampleLoadingDescriptor>();
+    final standardDescriptor =
+        loadingDescriptor.whereType<StandardExampleLoadingDescriptor>();

Review Comment:
   1. descriptor*s*
   2. This does not cover 4 other descriptor types, not to mention possibly future types.



##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -99,9 +101,12 @@ class SnippetEditingController extends ChangeNotifier {
   /// Creates an [ExampleLoadingDescriptor] that can recover the
   /// current content.
   ExampleLoadingDescriptor getLoadingDescriptor() {
-    // TODO: Return other classes for unchanged standard examples,
-    //  user-shared examples, and an empty editor,
-    //  https://github.com/apache/beam/issues/23252
+    if (codeController.fullText.isEmpty) {
+      return EmptyExampleLoadingDescriptor(sdk: sdk);
+    }
+    if (selectedExample != null && !isChanged) {
+      return StandardExampleLoadingDescriptor(path: _selectedExample!.path);

Review Comment:
   This could be other example sources. The best way to handle this is to store the original descriptor in this controller.



##########
playground/frontend/playground_components/test/src/controllers/snippet_editing_controllers_test.dart:
##########
@@ -0,0 +1,55 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:playground_components/src/controllers/snippet_editing_controller.dart';
+import 'package:playground_components/src/models/sdk.dart';
+
+import '../common/examples.dart';
+

Review Comment:
   `snippet_editing_controller_test.dart` without 's'.



##########
playground/frontend/playground_components/test/src/controllers/snippet_editing_controllers_test.dart:
##########
@@ -0,0 +1,55 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:playground_components/src/controllers/snippet_editing_controller.dart';
+import 'package:playground_components/src/models/sdk.dart';
+
+import '../common/examples.dart';
+
+void main() {
+  group(
+    'Snippet editing controllers',
+    () {
+      test(
+        'Returns standard descriptor if code has not been changed',
+        () {
+          final controller = SnippetEditingController(sdk: Sdk.python);
+          controller.selectedExample = exampleMock1;
+
+          final descriptor = controller.getLoadingDescriptor();
+
+          expect(descriptor.toJson(), {'example': 'SDK_PYTHON/Category/Name1'});
+        },
+      );
+
+      test(
+        'Returns content descriptor if code has been changed',
+        () {
+          final controller = SnippetEditingController(sdk: Sdk.python);
+          controller.selectedExample = exampleMock1;
+
+          controller.codeController.value = const TextEditingValue(text: 'ex4');
+          final descriptor = controller.getLoadingDescriptor();
+
+          expect(descriptor.toJson(), {
+            'sdk': 'python',
+            'content': 'ex4',
+            'name': 'Example X1',
+            'complexity': 'basic'

Review Comment:
   `,`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] Malarg commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
Malarg commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1034355929


##########
playground/frontend/lib/pages/embedded_playground/components/embedded_actions.dart:
##########
@@ -58,16 +59,26 @@ class EmbeddedActions extends StatelessWidget {
   void _openStandalonePlayground(PlaygroundController controller) {
     // The empty list forces the parsing of EmptyExampleLoadingDescriptor
     // and prevents the glimpse of the default catalog example.
+
+    final loadingDescriptor = controller.getLoadingDescriptor();
+    final contentDescriptor =
+        loadingDescriptor.whereType<ContentExampleLoadingDescriptor>();
+    final standardDescriptor =
+        loadingDescriptor.whereType<StandardExampleLoadingDescriptor>();

Review Comment:
   fixed



##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -99,9 +101,12 @@ class SnippetEditingController extends ChangeNotifier {
   /// Creates an [ExampleLoadingDescriptor] that can recover the
   /// current content.
   ExampleLoadingDescriptor getLoadingDescriptor() {
-    // TODO: Return other classes for unchanged standard examples,
-    //  user-shared examples, and an empty editor,
-    //  https://github.com/apache/beam/issues/23252
+    if (codeController.fullText.isEmpty) {
+      return EmptyExampleLoadingDescriptor(sdk: sdk);
+    }
+    if (selectedExample != null && !isChanged) {
+      return StandardExampleLoadingDescriptor(path: _selectedExample!.path);

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] Malarg commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
Malarg commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1035670984


##########
playground/frontend/lib/pages/embedded_playground/components/embedded_actions.dart:
##########
@@ -58,16 +59,24 @@ class EmbeddedActions extends StatelessWidget {
   void _openStandalonePlayground(PlaygroundController controller) {
     // The empty list forces the parsing of EmptyExampleLoadingDescriptor
     // and prevents the glimpse of the default catalog example.
+
+    final descriptor = controller.getLoadingDescriptor();
+    final urlDescriptor = descriptor.where((d) => d.canBePassedByUrl);
+    final jsDescriptor = descriptor.where((d) => !d.canBePassedByUrl);
+
+    final json = jsonEncode(urlDescriptor.toJson()['descriptors']);

Review Comment:
   fixed



##########
playground/frontend/playground_components/test/src/controllers/snippet_editing_controller_test.dart:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'package:flutter_test/flutter_test.dart';
+import 'package:playground_components/src/controllers/snippet_editing_controller.dart';
+import 'package:playground_components/src/models/sdk.dart';
+
+import '../common/examples.dart';
+
+void main() {
+  group(
+    'Snippet editing controller',
+    () {
+      test(
+        'Returns standard descriptor if code has not been changed',
+        () {
+          final controller = SnippetEditingController(sdk: Sdk.python);
+          controller.selectedExample = exampleMock1;
+
+          final descriptor = controller.getLoadingDescriptor();
+
+          expect(descriptor.toJson(), {'example': 'SDK_PYTHON/Category/Name1'});
+        },
+      );
+
+      test(
+        'Returns content descriptor if code has been changed',
+        () {
+          final controller = SnippetEditingController(sdk: Sdk.python);
+          controller.selectedExample = exampleMock1;
+
+          controller.codeController.value = const TextEditingValue(text: 'ex4');
+          final descriptor = controller.getLoadingDescriptor();
+
+          expect(descriptor.toJson(), {
+            'sdk': 'python',
+            'content': 'ex4',
+            'name': 'Example X1',
+            'complexity': 'basic',
+          });

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] codecov[bot] commented on pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #24283:
URL: https://github.com/apache/beam/pull/24283#issuecomment-1333344915

   # [Codecov](https://codecov.io/gh/apache/beam/pull/24283?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#24283](https://codecov.io/gh/apache/beam/pull/24283?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0123b2b) into [master](https://codecov.io/gh/apache/beam/commit/a04682a71a957ec35f876b345caa5dd37cd52ab5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a04682a) will **decrease** coverage by `0.09%`.
   > The diff coverage is `45.33%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #24283      +/-   ##
   ==========================================
   - Coverage   73.46%   73.37%   -0.10%     
   ==========================================
     Files         714      717       +3     
     Lines       96497    96918     +421     
   ==========================================
   + Hits        70896    71118     +222     
   - Misses      24279    24478     +199     
     Partials     1322     1322              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `82.99% <ø> (-0.20%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/24283?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/core/graph/coder/coder.go](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL2NvZGVyLmdv) | `85.50% <0.00%> (ø)` | |
   | [sdks/go/pkg/beam/core/runtime/exec/datasource.go](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9kYXRhc291cmNlLmdv) | `62.00% <0.00%> (ø)` | |
   | [sdks/go/pkg/beam/core/runtime/graphx/translate.go](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3RyYW5zbGF0ZS5nbw==) | `38.43% <ø> (ø)` | |
   | [sdks/go/pkg/beam/core/runtime/exec/translate.go](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy90cmFuc2xhdGUuZ28=) | `12.72% <17.94%> (ø)` | |
   | [sdks/go/pkg/beam/core/runtime/graphx/dataflow.go](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L2RhdGFmbG93Lmdv) | `53.93% <46.15%> (ø)` | |
   | [sdks/go/pkg/beam/coder.go](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb2Rlci5nbw==) | `54.95% <47.05%> (ø)` | |
   | [sdks/go/pkg/beam/core/runtime/exec/window.go](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy93aW5kb3cuZ28=) | `49.01% <50.00%> (ø)` | |
   | [sdks/go/pkg/beam/core/runtime/graphx/coder.go](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L2NvZGVyLmdv) | `51.91% <53.33%> (ø)` | |
   | [sdks/go/pkg/beam/core/runtime/exec/coder.go](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9jb2Rlci5nbw==) | `57.74% <66.66%> (ø)` | |
   | [sdks/go/pkg/beam/core/typex/fulltype.go](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3R5cGV4L2Z1bGx0eXBlLmdv) | `41.50% <69.23%> (ø)` | |
   | ... and [39 more](https://codecov.io/gh/apache/beam/pull/24283/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] Malarg commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
Malarg commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1034357699


##########
playground/frontend/playground_components/test/src/controllers/snippet_editing_controllers_test.dart:
##########
@@ -0,0 +1,55 @@
+import 'package:flutter_test/flutter_test.dart';

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] Malarg commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
Malarg commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1034356347


##########
playground/frontend/playground_components/test/src/controllers/snippet_editing_controllers_test.dart:
##########
@@ -0,0 +1,55 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:playground_components/src/controllers/snippet_editing_controller.dart';
+import 'package:playground_components/src/models/sdk.dart';
+
+import '../common/examples.dart';
+

Review Comment:
   fixed



##########
playground/frontend/playground_components/test/src/controllers/snippet_editing_controllers_test.dart:
##########
@@ -0,0 +1,55 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:playground_components/src/controllers/snippet_editing_controller.dart';
+import 'package:playground_components/src/models/sdk.dart';
+
+import '../common/examples.dart';
+
+void main() {
+  group(
+    'Snippet editing controllers',
+    () {
+      test(
+        'Returns standard descriptor if code has not been changed',
+        () {
+          final controller = SnippetEditingController(sdk: Sdk.python);
+          controller.selectedExample = exampleMock1;
+
+          final descriptor = controller.getLoadingDescriptor();
+
+          expect(descriptor.toJson(), {'example': 'SDK_PYTHON/Category/Name1'});
+        },
+      );
+
+      test(
+        'Returns content descriptor if code has been changed',
+        () {
+          final controller = SnippetEditingController(sdk: Sdk.python);
+          controller.selectedExample = exampleMock1;
+
+          controller.codeController.value = const TextEditingValue(text: 'ex4');
+          final descriptor = controller.getLoadingDescriptor();
+
+          expect(descriptor.toJson(), {
+            'sdk': 'python',
+            'content': 'ex4',
+            'name': 'Example X1',
+            'complexity': 'basic'

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] Malarg closed pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
Malarg closed pull request #24283: Example from embedded to standalone playground sent by path
URL: https://github.com/apache/beam/pull/24283


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] Malarg commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
Malarg commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1030090520


##########
playground/frontend/lib/pages/embedded_playground/components/embedded_actions.dart:
##########
@@ -58,16 +57,12 @@ class EmbeddedActions extends StatelessWidget {
   void _openStandalonePlayground(PlaygroundController controller) {
     // The empty list forces the parsing of EmptyExampleLoadingDescriptor
     // and prevents the glimpse of the default catalog example.
-    final window = html.window.open(
-      '/?$kExamplesParam=[]&$kSdkParam=${controller.sdk?.id}',
-      '',
-    );
 
-    javaScriptPostMessageRepeated(
-      window,
-      SetContentMessage(
-        descriptor: controller.getLoadingDescriptor(),
-      ),
+    var descriptors = controller.getLoadingDescriptor().toJson()['descriptors'];
+    var json = jsonEncode(descriptors);
+    html.window.open(
+      '/?$kExamplesParam=$json&$kSdkParam=${controller.sdk?.id}',
+      '',

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] github-actions[bot] commented on pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24283:
URL: https://github.com/apache/beam/pull/24283#issuecomment-1333328447

   Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
   
   R: @pabloem for label python.
   R: @apilloud for label java.
   R: @lostluck for label go.
   R: @Abacn for label build.
   R: @pabloem for label io.
   
   Available commands:
   - `stop reviewer notifications` - opt out of the automated review tooling
   - `remind me after tests pass` - tag the comment author after tests pass
   - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
   
   The PR bot will only process comments in the main thread (not review comments).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] alexeyinkin commented on a diff in pull request #24283: Example from embedded to standalone playground sent by path

Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24283:
URL: https://github.com/apache/beam/pull/24283#discussion_r1034478976


##########
playground/frontend/playground_components/lib/src/models/example.dart:
##########
@@ -49,8 +53,11 @@ class Example extends ExampleBase {
     required this.logs,
     required this.outputs,
     required this.source,
+    ExampleLoadingDescriptor? descriptor,
     this.graph,
-  }) : super(
+  })  : descriptor =
+            descriptor ?? StandardExampleLoadingDescriptor(path: example.path),

Review Comment:
   Second thoughts, it's better to not put the descriptor into the `Example` and just do `SnippetEditingController.setExample(example, descriptor: descriptor)`
   
   This is better because from the domain POV the descriptor is not a part of the example. And this simplifies a lot of things.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org