alexeyinkin commented on code in PR #23417: URL: https://github.com/apache/beam/pull/23417#discussion_r985418892
########## learning/tour-of-beam/frontend/lib/repositories/client/client.dart: ########## @@ -0,0 +1,26 @@ +/* + * 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 '../../models/content_tree.dart'; +import '../models/get_sdks_response.dart'; + +abstract class TobClient { Review Comment: We do have the same entity caches and clients in Playground. The difference is that in Playground we additionally have a repository layer between an entity cache and the abstract network client. It is a useful abstract for an entity cache to not be aware if it gets objects from the network or from a local DB. However, network is the only implementation in Playground so far, and until we have two implementations it's better to not add abstractions. For example, in Playground we have `CodeRespository` which has `runCode` method. It should be an action and not a repository intent, but it was modeled after `ExampleRepository` to just have the same number of abstractions. If we need a layer between an entity cache and the network, I would rather wait for a hands-on with local storage before adding an extra layer here just to see what are the differences in requirements for the implementations. So I would rather make Playground closer to the current model of ToB. ########## learning/tour-of-beam/frontend/lib/repositories/client/cloud_functions_client.dart: ########## @@ -0,0 +1,50 @@ +/* + * 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 'dart:convert'; + +import 'package:http/http.dart' as http; + +import '../../models/content_tree.dart'; +import '../models/get_sdks_response.dart'; +import 'client.dart'; + +class CloudFunctionsTobClient extends TobClient { + @override + Future<GetSdksResponse> getSdks() async { + final json = await http.get( + Uri.parse( + 'https://us-central1-tour-of-beam-2.cloudfunctions.net/getSdkList', Review Comment: Sure. It's just a temporary URL of a test backend. In production, we will have a deployment workflow, and the production backend URL will be emitted at some point during that workflow. When it is ready, we will decide how to capture it for the frontend. It may be a generated `.dart` file with constants (if the URL is more or less permanent) or a text file read at startup (if the URL may change with backend re-deployments). Without knowing the future infrastructure details, the best we can do is to extract the domain `us-central1-tour-of-beam-2.cloudfunctions.net` to some constant to not repeat it for two functions which is a too minor improvement. I suggest we do this in the next ToB PR which is already based on this unmerged one. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
