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/09/23 13:22:57 UTC
[GitHub] [beam] eantyshev opened a new pull request, #23349: [Tour Of Beam] API adjustments
eantyshev opened a new pull request, #23349:
URL: https://github.com/apache/beam/pull/23349
- [ ] Decouple SDK Id and Title, return Sdk objects in getSdkList response
- [ ] Return `title` instead of `name` in getContentTree response
- [ ] Return `id` instead of `unitId/moduleId` in getContentTree response
------------------------
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)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](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] eantyshev commented on a diff in pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
eantyshev commented on code in PR #23349:
URL: https://github.com/apache/beam/pull/23349#discussion_r982437891
##########
learning/tour-of-beam/backend/middleware.go:
##########
@@ -0,0 +1,79 @@
+// 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 (
+ "log"
+ "net/http"
+
+ tob "beam.apache.org/learning/tour-of-beam/backend/internal"
+)
+
+// Middleware-maker for setting a header
+// We also make this less generic: it works with HandlerFunc's
+// so that to be convertible to func(w http ResponseWriter, r *http.Request)
+// and be accepted by functions.HTTP.
+func AddHeader(header, value string) func(http.HandlerFunc) http.HandlerFunc {
+ return func(next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Add(header, value)
+ next(w, r)
+ }
+ }
+}
+
+// Middleware to check http method.
+func EnsureMethod(method string) func(http.HandlerFunc) http.HandlerFunc {
+ return func(next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ if r.Method == method {
+ next(w, r)
+ } else {
+ w.WriteHeader(http.StatusMethodNotAllowed)
+ }
+ }
+ }
+}
+
+// Helper common AIO middleware
+func Common(next http.HandlerFunc) http.HandlerFunc {
+ addContentType := AddHeader("Content-Type", "application/json")
+ addCORS := AddHeader("Access-Control-Allow-Origin", "*")
+ ensureGet := EnsureMethod(http.MethodGet)
+
+ return ensureGet(addCORS(addContentType(next)))
+}
+
+// HandleFunc enriched with sdk.
+type HandlerFuncWithSdk func(w http.ResponseWriter, r *http.Request, sdk tob.Sdk)
+
+// middleware to parse sdk query param and pass it as additional handler param.
+func ParseSdkParam(next HandlerFuncWithSdk) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ 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, "unknown sdk")
Review Comment:
Just realized that it makes no sense responding with SDK value to the client: it has just given it as a query parameter
This was triggered by the fact that GetSdkList changed signature from list[string] to list[SdkItem]
Keeping another version of GetSdkList for only this purpose seems like overkill
--
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] vchunikhin commented on pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
vchunikhin commented on PR #23349:
URL: https://github.com/apache/beam/pull/23349#issuecomment-1257539014
lgfm
@eantyshev hi!, look through that https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Commit/8063/console
PreCommit was failed.
--
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] damccorm merged pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
damccorm merged PR #23349:
URL: https://github.com/apache/beam/pull/23349
--
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 #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #23349:
URL: https://github.com/apache/beam/pull/23349#issuecomment-1256318497
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @Abacn for label build.
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] eantyshev commented on a diff in pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
eantyshev commented on code in PR #23349:
URL: https://github.com/apache/beam/pull/23349#discussion_r980183380
##########
learning/tour-of-beam/backend/docker-compose.yml:
##########
@@ -25,4 +25,4 @@ services:
- DATASTORE_LISTEN_ADDRESS=0.0.0.0:8081
ports:
- "8081:8081"
- command: --consistency=1.0
+ command: --consistency=1.0 --store-on-disk
Review Comment:
> --store-on-disk is a enabled by default not sure we need it here
I believe better to set it explicitly, provided we have to define other cmd options, as the default may change in future
> maybe we should try to set consistency to recommended 0.05 for development?
This setup is about CI/CD, we don't want to introduce undefined behavior here
--
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] eantyshev commented on pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
eantyshev commented on PR #23349:
URL: https://github.com/apache/beam/pull/23349#issuecomment-1257748604
That was a glitch in a CI infrastructure
--
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] olehborysevych commented on a diff in pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
olehborysevych commented on code in PR #23349:
URL: https://github.com/apache/beam/pull/23349#discussion_r979940780
##########
learning/tour-of-beam/backend/docker-compose.yml:
##########
@@ -25,4 +25,4 @@ services:
- DATASTORE_LISTEN_ADDRESS=0.0.0.0:8081
ports:
- "8081:8081"
- command: --consistency=1.0
+ command: --consistency=1.0 --store-on-disk
Review Comment:
@eantyshev maybe we should try to set consistency to recommended 0.05 for development?
--
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 #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #23349:
URL: https://github.com/apache/beam/pull/23349#issuecomment-1260587854
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
--
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] eantyshev commented on a diff in pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
eantyshev commented on code in PR #23349:
URL: https://github.com/apache/beam/pull/23349#discussion_r982437891
##########
learning/tour-of-beam/backend/middleware.go:
##########
@@ -0,0 +1,79 @@
+// 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 (
+ "log"
+ "net/http"
+
+ tob "beam.apache.org/learning/tour-of-beam/backend/internal"
+)
+
+// Middleware-maker for setting a header
+// We also make this less generic: it works with HandlerFunc's
+// so that to be convertible to func(w http ResponseWriter, r *http.Request)
+// and be accepted by functions.HTTP.
+func AddHeader(header, value string) func(http.HandlerFunc) http.HandlerFunc {
+ return func(next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Add(header, value)
+ next(w, r)
+ }
+ }
+}
+
+// Middleware to check http method.
+func EnsureMethod(method string) func(http.HandlerFunc) http.HandlerFunc {
+ return func(next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ if r.Method == method {
+ next(w, r)
+ } else {
+ w.WriteHeader(http.StatusMethodNotAllowed)
+ }
+ }
+ }
+}
+
+// Helper common AIO middleware
+func Common(next http.HandlerFunc) http.HandlerFunc {
+ addContentType := AddHeader("Content-Type", "application/json")
+ addCORS := AddHeader("Access-Control-Allow-Origin", "*")
+ ensureGet := EnsureMethod(http.MethodGet)
+
+ return ensureGet(addCORS(addContentType(next)))
+}
+
+// HandleFunc enriched with sdk.
+type HandlerFuncWithSdk func(w http.ResponseWriter, r *http.Request, sdk tob.Sdk)
+
+// middleware to parse sdk query param and pass it as additional handler param.
+func ParseSdkParam(next HandlerFuncWithSdk) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ 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, "unknown sdk")
Review Comment:
Just realized that it makes no sense responding with SDK value to the client: it has just given it as a query parameter
This was triggered by the fact that GetSdkList changed return value type from list[string] to list[SdkItem]
Keeping another version of GetSdkList for only this purpose seems like overkill
--
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] olehborysevych commented on pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
olehborysevych commented on PR #23349:
URL: https://github.com/apache/beam/pull/23349#issuecomment-1260586599
R: @damccorm
--
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] damccorm commented on pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #23349:
URL: https://github.com/apache/beam/pull/23349#issuecomment-1261047775
Looks like we need to run go fmt for precommits to pass
--
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] eantyshev commented on a diff in pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
eantyshev commented on code in PR #23349:
URL: https://github.com/apache/beam/pull/23349#discussion_r982437891
##########
learning/tour-of-beam/backend/middleware.go:
##########
@@ -0,0 +1,79 @@
+// 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 (
+ "log"
+ "net/http"
+
+ tob "beam.apache.org/learning/tour-of-beam/backend/internal"
+)
+
+// Middleware-maker for setting a header
+// We also make this less generic: it works with HandlerFunc's
+// so that to be convertible to func(w http ResponseWriter, r *http.Request)
+// and be accepted by functions.HTTP.
+func AddHeader(header, value string) func(http.HandlerFunc) http.HandlerFunc {
+ return func(next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Add(header, value)
+ next(w, r)
+ }
+ }
+}
+
+// Middleware to check http method.
+func EnsureMethod(method string) func(http.HandlerFunc) http.HandlerFunc {
+ return func(next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ if r.Method == method {
+ next(w, r)
+ } else {
+ w.WriteHeader(http.StatusMethodNotAllowed)
+ }
+ }
+ }
+}
+
+// Helper common AIO middleware
+func Common(next http.HandlerFunc) http.HandlerFunc {
+ addContentType := AddHeader("Content-Type", "application/json")
+ addCORS := AddHeader("Access-Control-Allow-Origin", "*")
+ ensureGet := EnsureMethod(http.MethodGet)
+
+ return ensureGet(addCORS(addContentType(next)))
+}
+
+// HandleFunc enriched with sdk.
+type HandlerFuncWithSdk func(w http.ResponseWriter, r *http.Request, sdk tob.Sdk)
+
+// middleware to parse sdk query param and pass it as additional handler param.
+func ParseSdkParam(next HandlerFuncWithSdk) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ 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, "unknown sdk")
Review Comment:
Just realized that it makes no sense responding with SDK value to the client: it has just given it as a query parameter
--
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] olehborysevych commented on pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
olehborysevych commented on PR #23349:
URL: https://github.com/apache/beam/pull/23349#issuecomment-1257974659
lgtm
--
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] damccorm commented on a diff in pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #23349:
URL: https://github.com/apache/beam/pull/23349#discussion_r982428893
##########
learning/tour-of-beam/backend/internal/sdk.go:
##########
@@ -19,33 +19,52 @@ type Sdk string
const (
SDK_UNDEFINED Sdk = ""
- SDK_GO Sdk = "Go"
- SDK_PYTHON Sdk = "Python"
- SDK_JAVA Sdk = "Java"
- SDK_SCIO Sdk = "SCIO"
+ SDK_GO Sdk = "go"
+ SDK_PYTHON Sdk = "python"
+ SDK_JAVA Sdk = "java"
+ SDK_SCIO Sdk = "scio"
)
func (s Sdk) String() string {
return string(s)
}
-// Parse sdk from string names, f.e. "Java" -> Sdk.GO_JAVA
+func (s Sdk) Title() string {
Review Comment:
Could you add a function header comment?
##########
learning/tour-of-beam/backend/middleware.go:
##########
@@ -0,0 +1,79 @@
+// 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 (
+ "log"
+ "net/http"
+
+ tob "beam.apache.org/learning/tour-of-beam/backend/internal"
+)
+
+// Middleware-maker for setting a header
+// We also make this less generic: it works with HandlerFunc's
+// so that to be convertible to func(w http ResponseWriter, r *http.Request)
+// and be accepted by functions.HTTP.
+func AddHeader(header, value string) func(http.HandlerFunc) http.HandlerFunc {
+ return func(next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Add(header, value)
+ next(w, r)
+ }
+ }
+}
+
+// Middleware to check http method.
+func EnsureMethod(method string) func(http.HandlerFunc) http.HandlerFunc {
+ return func(next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ if r.Method == method {
+ next(w, r)
+ } else {
+ w.WriteHeader(http.StatusMethodNotAllowed)
+ }
+ }
+ }
+}
+
+// Helper common AIO middleware
+func Common(next http.HandlerFunc) http.HandlerFunc {
+ addContentType := AddHeader("Content-Type", "application/json")
+ addCORS := AddHeader("Access-Control-Allow-Origin", "*")
+ ensureGet := EnsureMethod(http.MethodGet)
+
+ return ensureGet(addCORS(addContentType(next)))
+}
+
+// HandleFunc enriched with sdk.
+type HandlerFuncWithSdk func(w http.ResponseWriter, r *http.Request, sdk tob.Sdk)
+
+// middleware to parse sdk query param and pass it as additional handler param.
+func ParseSdkParam(next HandlerFuncWithSdk) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ 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, "unknown sdk")
Review Comment:
Any reason we took out the message about available SDKs here? (`message := fmt.Sprintf("Sdk not in: %v", tob.SdksList())`)
##########
learning/tour-of-beam/backend/internal/sdk.go:
##########
@@ -19,33 +19,52 @@ type Sdk string
const (
SDK_UNDEFINED Sdk = ""
- SDK_GO Sdk = "Go"
- SDK_PYTHON Sdk = "Python"
- SDK_JAVA Sdk = "Java"
- SDK_SCIO Sdk = "SCIO"
+ SDK_GO Sdk = "go"
+ SDK_PYTHON Sdk = "python"
+ SDK_JAVA Sdk = "java"
+ SDK_SCIO Sdk = "scio"
)
func (s Sdk) String() string {
return string(s)
}
-// Parse sdk from string names, f.e. "Java" -> Sdk.GO_JAVA
+func (s Sdk) Title() string {
+ switch s {
+ case SDK_GO:
+ return "Go"
Review Comment:
Any reason we capitalize names here and leave them all lowercase below?
--
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] damccorm commented on a diff in pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #23349:
URL: https://github.com/apache/beam/pull/23349#discussion_r982523296
##########
learning/tour-of-beam/backend/internal/sdk.go:
##########
@@ -19,33 +19,52 @@ type Sdk string
const (
SDK_UNDEFINED Sdk = ""
- SDK_GO Sdk = "Go"
- SDK_PYTHON Sdk = "Python"
- SDK_JAVA Sdk = "Java"
- SDK_SCIO Sdk = "SCIO"
+ SDK_GO Sdk = "go"
+ SDK_PYTHON Sdk = "python"
+ SDK_JAVA Sdk = "java"
+ SDK_SCIO Sdk = "scio"
)
func (s Sdk) String() string {
return string(s)
}
-// Parse sdk from string names, f.e. "Java" -> Sdk.GO_JAVA
+func (s Sdk) Title() string {
+ switch s {
+ case SDK_GO:
+ return "Go"
Review Comment:
SGTM
--
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] damccorm commented on a diff in pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #23349:
URL: https://github.com/apache/beam/pull/23349#discussion_r982524703
##########
learning/tour-of-beam/backend/middleware.go:
##########
@@ -0,0 +1,79 @@
+// 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 (
+ "log"
+ "net/http"
+
+ tob "beam.apache.org/learning/tour-of-beam/backend/internal"
+)
+
+// Middleware-maker for setting a header
+// We also make this less generic: it works with HandlerFunc's
+// so that to be convertible to func(w http ResponseWriter, r *http.Request)
+// and be accepted by functions.HTTP.
+func AddHeader(header, value string) func(http.HandlerFunc) http.HandlerFunc {
+ return func(next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Add(header, value)
+ next(w, r)
+ }
+ }
+}
+
+// Middleware to check http method.
+func EnsureMethod(method string) func(http.HandlerFunc) http.HandlerFunc {
+ return func(next http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ if r.Method == method {
+ next(w, r)
+ } else {
+ w.WriteHeader(http.StatusMethodNotAllowed)
+ }
+ }
+ }
+}
+
+// Helper common AIO middleware
+func Common(next http.HandlerFunc) http.HandlerFunc {
+ addContentType := AddHeader("Content-Type", "application/json")
+ addCORS := AddHeader("Access-Control-Allow-Origin", "*")
+ ensureGet := EnsureMethod(http.MethodGet)
+
+ return ensureGet(addCORS(addContentType(next)))
+}
+
+// HandleFunc enriched with sdk.
+type HandlerFuncWithSdk func(w http.ResponseWriter, r *http.Request, sdk tob.Sdk)
+
+// middleware to parse sdk query param and pass it as additional handler param.
+func ParseSdkParam(next HandlerFuncWithSdk) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ 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, "unknown sdk")
Review Comment:
That's fair, sounds good
--
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] olehborysevych commented on a diff in pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
olehborysevych commented on code in PR #23349:
URL: https://github.com/apache/beam/pull/23349#discussion_r979949315
##########
learning/tour-of-beam/backend/docker-compose.yml:
##########
@@ -25,4 +25,4 @@ services:
- DATASTORE_LISTEN_ADDRESS=0.0.0.0:8081
ports:
- "8081:8081"
- command: --consistency=1.0
+ command: --consistency=1.0 --store-on-disk
Review Comment:
@eantyshev --store-on-disk is a enabled by default not sure we need it here
--
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] eantyshev commented on a diff in pull request #23349: [Tour Of Beam] API adjustments
Posted by GitBox <gi...@apache.org>.
eantyshev commented on code in PR #23349:
URL: https://github.com/apache/beam/pull/23349#discussion_r980183380
##########
learning/tour-of-beam/backend/docker-compose.yml:
##########
@@ -25,4 +25,4 @@ services:
- DATASTORE_LISTEN_ADDRESS=0.0.0.0:8081
ports:
- "8081:8081"
- command: --consistency=1.0
+ command: --consistency=1.0 --store-on-disk
Review Comment:
> --store-on-disk is a enabled by default not sure we need it here
I believe better to set it explicitly, provided we have to define other cmd options, as the default may change in future
> maybe we should try to set consistency to recommended 0.05 for development?
This setup is about CI/CD, we don't want to introduce undefined behavior here
--
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