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/08/29 15:33:52 UTC
[GitHub] [beam] damccorm commented on a diff in pull request #22556: [akvelon][tour-of-beam] backend bootstraps
damccorm commented on code in PR #22556:
URL: https://github.com/apache/beam/pull/22556#discussion_r957445818
##########
.github/workflows/tour_of_beam_backend.yml:
##########
@@ -0,0 +1,59 @@
+# 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.
+
+# To learn more about GitHub Actions in Apache Beam check the CI.md
+
+name: Go tests
+
+on:
+ push:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ pull_request:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ paths: ['learning/tour-of-beam/backend/**']
+
+jobs:
+ checks:
+ runs-on: ubuntu-latest
+ defaults:
+ run:
+ working-directory: ./learning/tour-of-beam/backend
+ steps:
+ - uses: actions/checkout@v3
+ - uses: actions/setup-go@v3
+ with:
+ go-version: '1.16'
+ - name: Run fmt
+ run: |
+ go fmt ./...
+ git diff-index --quiet HEAD || (echo "Run go fmt before checking in changes" && exit 1)
+
+ - name: Run vet
+ run: go vet ./...
+
+ - name: Run test
+ run: go test -v ./...
+
+ - name: golangci-lint
+ uses: golangci/golangci-lint-action@v3
+ with:
+ version: v1.49.0
+ working-directory: learning/tour-of-beam/backend
+ skip-cache: true
Review Comment:
Is there a reason to skip-cache?
##########
.github/workflows/tour_of_beam_backend.yml:
##########
@@ -0,0 +1,59 @@
+# 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.
+
+# To learn more about GitHub Actions in Apache Beam check the CI.md
+
+name: Go tests
+
+on:
+ push:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ pull_request:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ paths: ['learning/tour-of-beam/backend/**']
+
+jobs:
+ checks:
+ runs-on: ubuntu-latest
+ defaults:
+ run:
+ working-directory: ./learning/tour-of-beam/backend
+ steps:
+ - uses: actions/checkout@v3
+ - uses: actions/setup-go@v3
+ with:
+ go-version: '1.16'
Review Comment:
Any reason we're pinning to 1.16 instead of 1.18 (latest)?
##########
.github/workflows/tour_of_beam_backend.yml:
##########
@@ -0,0 +1,59 @@
+# 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.
+
+# To learn more about GitHub Actions in Apache Beam check the CI.md
+
+name: Go tests
Review Comment:
```suggestion
name: Tour of Beam Go tests
```
Go tests in the context of this repo suggests that we're testing the Go SDK, so I'd prefer a more verbose name.
##########
learning/tour-of-beam/backend/README.md:
##########
@@ -0,0 +1,56 @@
+<!--
+ Licensed 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.
+-->
+
+## Tour Of Beam Backend
+
+Backend provides the learning content tree for a given SDK,
+and currently logged-in user's snippets and progress.
+Currently it supports Java, Python, and Go Beam SDK.
+
+It is comprised of several Cloud Functions, with Firerstore in Datastore mode as a storage.
+* list-sdks
+* get-content-tree?sdk=(Java|Go|Python)
+* get-unit-content?unitId=<id>
+TODO: add response schemas
+TODO: add save functions info
+TODO: add user token info
Review Comment:
Do we have issues tracking each of these? If not, don't worry about it, if we do it would be nice to include them as `TODO(https://github.com/apache/beam/issues/XYZ): ...`
##########
.github/workflows/tour_of_beam_backend.yml:
##########
@@ -0,0 +1,59 @@
+# 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.
+
+# To learn more about GitHub Actions in Apache Beam check the CI.md
+
+name: Go tests
+
+on:
+ push:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ pull_request:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ paths: ['learning/tour-of-beam/backend/**']
+
+jobs:
+ checks:
+ runs-on: ubuntu-latest
+ defaults:
+ run:
+ working-directory: ./learning/tour-of-beam/backend
+ steps:
+ - uses: actions/checkout@v3
+ - uses: actions/setup-go@v3
+ with:
+ go-version: '1.16'
Review Comment:
I mean generally for the project, not just in CI
##########
learning/tour-of-beam/backend/function.go:
##########
@@ -0,0 +1,109 @@
+// 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
+
+const (
+ BAD_FORMAT = "BAD_FORMAT"
+ INTERNAL_ERROR = "INTERNAL_ERROR"
+)
+
+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)
+
+}
+
+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) {
+ if r.Method != "GET" {
+ w.WriteHeader(http.StatusMethodNotAllowed)
+ return
+ }
+ w.Header().Add("Content-Type", "application/json")
+ fmt.Fprint(w, `{"names": ["Java", "Python", "Go"]}`)
+}
+
+func getContentTree(w http.ResponseWriter, r *http.Request) {
+ w.Header().Add("Content-Type", "application/json")
+ if r.Method != "GET" {
+ w.WriteHeader(http.StatusMethodNotAllowed)
+ return
+ }
+
+ sdkStr := r.URL.Query().Get("sdk")
+ sdk := tob.ParseSdk(sdkStr)
+ if sdk == tob.SDK_UNDEFINED {
+ log.Printf("Bad sdk: %v", sdkStr)
+ finalizeErrResponse(w, http.StatusBadRequest, BAD_FORMAT, "Bad sdk")
Review Comment:
Could we add the supported sdks here along with the Bad sdk message?
##########
learning/tour-of-beam/backend/internal/fs_content/load.go:
##########
@@ -0,0 +1,210 @@
+// 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 fs_content
+
+import (
+ "fmt"
+ "io/fs"
+ "io/ioutil"
+ "log"
+ "os"
+ "path/filepath"
+ "regexp"
+
+ tob "beam.apache.org/learning/tour-of-beam/backend/internal"
+)
+
+const (
+ contentInfoYaml = "content-info.yaml"
+ moduleInfoYaml = "module-info.yaml"
+ groupInfoYaml = "group-info.yaml"
+ unitInfoYaml = "unit-info.yaml"
+
+ descriptionMd = "description.md"
+ hintMdRegexp = "hint[0-9]*.md"
+)
+
+type learningPathInfo struct {
+ Sdk string
+ Content []string `yaml:"content"`
+}
+
+type learningModuleInfo struct {
+ Id string
+ Name string
+ Complexity string
+ Content []string `yaml:"content"`
+}
+
+type learningGroupInfo struct {
+ Name string
+ Content []string `yaml:"content"`
+}
+
+type learningUnitInfo struct {
+ Id string
+ Name string
+ TaskName string
+ SolutionName string
+}
+
+// Watch for duplicate ids. Not thread-safe!
+type IdsWatcher struct {
Review Comment:
Nit: This can probably be private (`idsWatcher`), right? It is pretty tightly coupled with loading content.
##########
learning/tour-of-beam/backend/function.go:
##########
@@ -0,0 +1,109 @@
+// 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
+
+const (
+ BAD_FORMAT = "BAD_FORMAT"
+ INTERNAL_ERROR = "INTERNAL_ERROR"
+)
+
+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)
+
+}
Review Comment:
```suggestion
functions.HTTP("getContentTree", getContentTree)
}
```
Nit - trailing space
##########
.github/workflows/tour_of_beam_backend.yml:
##########
@@ -0,0 +1,59 @@
+# 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.
+
+# To learn more about GitHub Actions in Apache Beam check the CI.md
+
+name: Go tests
+
+on:
+ push:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ pull_request:
+ branches: ['master', 'release-*']
+ tags: 'v*'
+ paths: ['learning/tour-of-beam/backend/**']
+
+jobs:
+ checks:
+ runs-on: ubuntu-latest
+ defaults:
+ run:
+ working-directory: ./learning/tour-of-beam/backend
+ steps:
+ - uses: actions/checkout@v3
+ - uses: actions/setup-go@v3
+ with:
+ go-version: '1.16'
Review Comment:
Ah, I think I found my answer - its because Google Cloud Functions don't support 1.18, right? https://cloud.google.com/functions/docs/concepts/go-runtime
##########
learning/tour-of-beam/backend/internal/storage/datastore.go:
##########
@@ -0,0 +1,231 @@
+// 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 {
+ Client *datastore.Client
+}
+
+// Query modules structure and content (recursively).
+func (d *DatastoreDb) collectModules(ctx context.Context, tx *datastore.Transaction,
+ rootKey *datastore.Key,
+) ([]tob.Module, error) {
+ // Custom index.yaml should be applied for this query to work
+ // (Ancestor + Order)
+ modules := make([]tob.Module, 0)
+ var tbMods []TbLearningModule
+ queryModules := datastore.NewQuery(TbLearningModuleKind).
+ Namespace(PgNamespace).
+ Ancestor(rootKey).
+ Order("order").
+ Transaction(tx)
+ _, err := d.Client.GetAll(ctx, queryModules, &tbMods)
+ if err != nil {
+ return modules, fmt.Errorf("error querying modules for %v: %w", rootKey, err)
+ }
+
+ for _, tbMod := range tbMods {
+ mod := tob.Module{Id: tbMod.Id, Name: tbMod.Name, Complexity: tbMod.Complexity}
+ mod.Nodes, err = d.collectNodes(ctx, tx, tbMod.Key, 0)
+ if err != nil {
+ return modules, err
+ }
+ modules = append(modules, mod)
+ }
+ return modules, nil
+}
+
+// Get a group recursively.
+// Params:
+// - parentKey
+// - level: depth of a node's children
+// Recursively query/collect for each subgroup key, with level = level + 1.
+func (d *DatastoreDb) collectNodes(ctx context.Context, tx *datastore.Transaction,
+ parentKey *datastore.Key, level int,
+) (nodes []tob.Node, err error) {
+ var tbNodes []TbLearningNode
+
+ // Custom index.yaml should be applied for this query to work
+ queryNodes := datastore.NewQuery(TbLearningNodeKind).
+ Namespace(PgNamespace).
+ Ancestor(parentKey).
+ FilterField("level", "=", level).
+ Project("type", "id", "name").
+ Order("order").
+ Transaction(tx)
+ if _, err = d.Client.GetAll(ctx, queryNodes, &tbNodes); err != nil {
+ return nodes, fmt.Errorf("getting children of node %v: %w", parentKey, err)
+ }
+
+ // traverse the nodes which are groups, with level=level+1
+ nodes = make([]tob.Node, 0, len(tbNodes))
+ for _, tbNode := range tbNodes {
+ node := FromDatastoreNode(tbNode)
+ if node.Type == tob.NODE_GROUP {
+ node.Group.Nodes, err = d.collectNodes(ctx, tx, tbNode.Key, level+1)
+ }
+ if err != nil {
+ return nodes, err
+ }
+ nodes = append(nodes, node)
+ }
+
+ return nodes, nil
+}
+
+// Get learning content tree for SDK.
+func (d *DatastoreDb) GetContentTree(ctx context.Context, sdk tob.Sdk) (tree tob.ContentTree, err error) {
+ var tbLP TbLearningPath
+ tree.Sdk = sdk
+
+ _, err = d.Client.RunInTransaction(ctx, func(tx *datastore.Transaction) error {
+ rootKey := pgNameKey(TbLearningPathKind, sdkToKey(sdk), nil)
+ if err := d.Client.Get(ctx, rootKey, &tbLP); err != nil {
+ return fmt.Errorf("error querying learning_path: %w", err)
+ }
+ tree.Modules, err = d.collectModules(ctx, tx, rootKey)
+ if err != nil {
+ return err
+ }
+ return nil
+ }, datastore.ReadOnly)
+
+ return tree, err
+}
+
+// Helper to clear all ToB Datastore entities related to a particular SDK
+// They have one common ancestor key in tb_learning_path.
+func (d *DatastoreDb) clearContentTree(ctx context.Context, tx *datastore.Transaction, sdk tob.Sdk) error {
+ rootKey := pgNameKey(TbLearningPathKind, sdkToKey(sdk), nil)
+ q := datastore.NewQuery("").
+ Namespace(PgNamespace).
+ Ancestor(rootKey).
+ KeysOnly().
+ Transaction(tx)
+ keys, err := d.Client.GetAll(ctx, q, nil)
+ if err != nil {
+ return err
+ }
+
+ for _, key := range keys {
+ log.Println("deleting ", key)
+ }
+
+ err = tx.DeleteMulti(keys)
+ if err != nil {
+ return err
+ }
+ return tx.Delete(rootKey)
+}
+
+// Serialize a content tree to Datastore.
+func (d *DatastoreDb) saveContentTree(tx *datastore.Transaction, tree *tob.ContentTree) error {
+ sdk := tree.Sdk
+
+ saveUnit := func(unit *tob.Unit, order, level int, parentKey *datastore.Key) error {
+ unitKey := datastoreKey(TbLearningNodeKind, tree.Sdk, unit.Id, parentKey)
+ _, err := tx.Put(unitKey, MakeUnitNode(unit, order, level))
+ if err != nil {
+ return fmt.Errorf("failed to put unit: %w", err)
+ }
+ return nil
+ }
+
+ // transaction-wide autoincremented Id
+ // could have used numericID keys, if there was no transaction:
+ // incomplete keys are resolved after Tx commit, and
+ // we need to reference them in child nodes
+ var groupId int = 0
+ genGroupKey := func(parentKey *datastore.Key) *datastore.Key {
+ groupId++
+ return datastoreKey(TbLearningNodeKind,
+ tree.Sdk, fmt.Sprintf("group%v", groupId), parentKey)
+ }
+
+ var saveNode func(tob.Node, int, int, *datastore.Key) error
+ saveGroup := func(group *tob.Group, order, level int, parentKey *datastore.Key) error {
+ groupKey := genGroupKey(parentKey)
+ if _, err := tx.Put(groupKey, MakeGroupNode(group, order, level)); err != nil {
+ return fmt.Errorf("failed to put group: %w", err)
+ }
+ for order, node := range group.Nodes {
+ if err := saveNode(node, order, level+1, groupKey); err != nil {
+ return err
+ }
+ }
+ return nil
+ }
+
+ saveNode = func(node tob.Node, order, level int, parentKey *datastore.Key) error {
+ if node.Type == tob.NODE_UNIT {
+ return saveUnit(node.Unit, order, level, parentKey)
+ } else if node.Type == tob.NODE_GROUP {
+ return saveGroup(node.Group, order, level, parentKey)
+ }
+
+ panic("Unknown node type")
Review Comment:
Should this be an error instead of a panic?
--
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