vchunikhin commented on code in PR #22556: URL: https://github.com/apache/beam/pull/22556#discussion_r954850117
########## learning/tour-of-beam/backend/internal/storage/image/Dockerfile: ########## @@ -0,0 +1,32 @@ +# 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. + + +# Version. Can change in build progress +ARG GCLOUD_SDK_VERSION=397.0.0-emulators + +# Use google cloud sdk +FROM google/cloud-sdk:$GCLOUD_SDK_VERSION + +# Volume to persist Datastore data +VOLUME /opt/data + +# RUN mkdir -p /opt/data/WEB-INF +COPY index.yaml /opt/data/WEB-INF/index.yaml Review Comment: Please, don't repeat mistakes from playground service :) Please minimize the number of layers according to best practices https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#minimize-the-number-of-layers ########## learning/tour-of-beam/backend/internal/storage/datastore.go: ########## @@ -0,0 +1,246 @@ +// 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. + +package storage + +import ( + "context" + "fmt" + "log" + + tob "beam.apache.org/learning/tour-of-beam/backend/internal" + "cloud.google.com/go/datastore" +) + +type DatastoreDb struct { Review Comment: I recommend you to create interface, e.g Storage where you list all methods for learning materials. Probably, in the future you will want to change the kind of storage ########## learning/tour-of-beam/backend/cmd/main.go: ########## @@ -0,0 +1,36 @@ +// 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. + +package main + +import ( + "log" + "os" + + // Blank-import the function package so the init() runs + _ "beam.apache.org/learning/tour-of-beam/backend" + "github.com/GoogleCloudPlatform/functions-framework-go/funcframework" +) + +func main() { + // Use PORT environment variable, or default to 8080. + port := "8080" + if envPort := os.Getenv("PORT"); envPort != "" { Review Comment: I think the best way here it is to create new package "environment" and develop all methods there to get variables ########## learning/tour-of-beam/backend/cmd/ci_cd/ci_cd.go: ########## @@ -0,0 +1,51 @@ +// 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. + +package main + +import ( + "context" + "fmt" + "log" + "os" + + "beam.apache.org/learning/tour-of-beam/backend/internal/fs_content" + "beam.apache.org/learning/tour-of-beam/backend/internal/storage" + "cloud.google.com/go/datastore" +) + +var repo storage.Iface + +func init() { + client, err := datastore.NewClient(context.Background(), "") + if err != nil { + log.Fatalf("new datastore client: %v", err) Review Comment: Is it ok that you don't handle this case and just run code further? ########## learning/tour-of-beam/backend/function.go: ########## @@ -0,0 +1,93 @@ +// 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. + +package tob + +import ( + "context" + "encoding/json" + "fmt" + "log" + "net/http" + "os" + + tob "beam.apache.org/learning/tour-of-beam/backend/internal" + "beam.apache.org/learning/tour-of-beam/backend/internal/service" + "beam.apache.org/learning/tour-of-beam/backend/internal/storage" + + "cloud.google.com/go/datastore" + "github.com/GoogleCloudPlatform/functions-framework-go/functions" +) + +var svc service.IContent + +func init() { + // dependencies + // required: + // * TOB_MOCK: respond with static samples + // OR + // * DATASTORE_PROJECT_ID: cloud project id + // optional: + // * DATASTORE_EMULATOR_HOST: emulator host/port (ex. 0.0.0.0:8888) + if os.Getenv("TOB_MOCK") > "" { + svc = &service.Mock{} + } else { + client, err := datastore.NewClient(context.Background(), "") + if err != nil { + log.Fatalf("new datastore client: %v", err) + } + svc = &service.Svc{Repo: &storage.DatastoreDb{Client: client}} + } + + // functions framework + functions.HTTP("sdkList", sdkList) + functions.HTTP("getContentTree", getContentTree) + //functions.HTTP("getUnitContent", getUnitContent) +} + +func finalizeErrResponse(w http.ResponseWriter, status int, code, message string) { + w.WriteHeader(status) + resp := tob.CodeMessage{Code: code, Message: message} + _ = json.NewEncoder(w).Encode(resp) +} + +func sdkList(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, `{"names": ["Java", "Python", "Go"]}`) +} + +func getContentTree(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Content-Type", "application/json") Review Comment: I think golang has constants for this header...I'm not sure, check it i mean smth like that https://pkg.go.dev/github.com/go-http-utils/headers ########## learning/tour-of-beam/backend/internal/storage/schema.go: ########## @@ -0,0 +1,99 @@ +// 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. + +package storage + +import ( + tob "beam.apache.org/learning/tour-of-beam/backend/internal" + "cloud.google.com/go/datastore" +) + +// ToB Datastore schema +// - Content tree has a root entity in tb_learning_path, +// descendant entities in tb_learning_module/group/unit have it as a common ancestor +// - learning path consists of modules, modules consist of either groups or units +// - A group can have only 1 type of children: only units or only groups +// - Ordering is established by "order" property +// - To limit ancestor queries by only first-level descendants, "level" property is used + +const ( + PgNamespace = "Playground" Review Comment: Oh.. I remember that :) First i did the same things, namely, wrote constants in the specific package. However, i had package cycling in the future. I moved these constants to the separate package as constants and use them anywhere where it is necessary -- 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]
