[GitHub] [beam] olehborysevych commented on a diff in pull request #24818: [Playground] Add an option for Datastore namespace value for ci_cd.py script

[GitBox <gi...@apache.org>]

olehborysevych commented on code in PR #24818:
URL: https://github.com/apache/beam/pull/24818#discussion_r1062383918


##########
playground/infrastructure/ci_cd.py:
##########
@@ -43,6 +43,12 @@
     "beam examples/tests/katas and their outputs on the GCD",
     choices=[Config.CI_STEP_NAME, Config.CD_STEP_NAME],
 )
+parser.add_argument(

Review Comment:
   @TSultanov  could you please update playground/README.md with info regarding new script argument



##########
playground/infrastructure/datastore_client.py:
##########
@@ -44,10 +44,10 @@ class DatastoreClient:
 
     _datastore_client: datastore.Client
 
-    def __init__(self):
+    def __init__(self, namespace: str):
         self._check_envs()
         self._datastore_client = datastore.Client(
-            namespace=DatastoreProps.NAMESPACE, project=Config.GOOGLE_CLOUD_PROJECT
+            namespace=namespace, project=Config.GOOGLE_CLOUD_PROJECT

Review Comment:
   I'm not comfortable that we have two related properties of the datastore client but they are obtained from different sources. Maybe it's not too late to reconsider this approach and have both as env variables or arguments? @TSultanov 



##########
playground/infrastructure/config.py:
##########
@@ -113,9 +114,9 @@ class DatastoreProps:
 class RepoProps:
     REPO_DATASETS_PATH = "../backend/datasets"
 
+

Review Comment:
   please revert extra line



-- 
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