damccorm commented on code in PR #24751:
URL: https://github.com/apache/beam/pull/24751#discussion_r1055829813
##########
playground/backend/internal/db/datastore/datastore_db.go:
##########
@@ -519,27 +521,26 @@ func rollback(tx *datastore.Transaction) {
}
}
-func getEntities[V entity.DatastoreEntity](tx *datastore.Transaction, keys
[]*datastore.Key) ([]*V, error) {
- var entitiesWithNils = make([]*V, len(keys))
- entities := make([]*V, 0)
+// generic wrapper around GetMulti & filtering nil elements
+func getEntities[V any](tx *datastore.Transaction, keys []*datastore.Key)
([]*V, error) {
+ entitiesWithNils := make([]*V, len(keys))
+ entitiesNotNil := make([]*V, 0)
if err := tx.GetMulti(keys, entitiesWithNils); err != nil {
if errorsVal, ok := err.(datastore.MultiError); ok {
- for _, errVal := range errorsVal {
- if errors.Is(datastore.ErrNoSuchEntity, errVal)
{
Review Comment:
Is there a reason we no longer need this error check? I don't quite follow
this change
##########
playground/infrastructure/helper.py:
##########
@@ -98,11 +99,17 @@ def find_examples(root_dir: str, subdirs: List[str], sdk:
SdkEnum) -> List[Examp
for filename in files:
filepath = os.path.join(root, filename)
try:
- example = _load_example(
- filename=filename, filepath=filepath, sdk=sdk
- )
- if example is not None:
- examples.append(example)
+ try:
+ example = _load_example(
+ filename=filename, filepath=filepath, sdk=sdk
+ )
+ if example is not None:
+ examples.append(example)
+ except pydantic.ValidationError as err:
+ if len(err.errors()) > 1:
+ raise
+ if err.errors()[0]["msg"] == "multifile is True but no
files defined":
+ logging.warning("incomplete multifile example
ignored %s", filepath)
Review Comment:
Should we raise an exception in the else clause here? Are there other cases
where we want to ignore an error?
##########
playground/infrastructure/ci_cd.py:
##########
@@ -88,9 +88,6 @@ def _run_ci_cd(step: str, raw_sdk: str, origin: Origin,
subdirs: List[str]):
logging.info("Finish of searching Playground examples")
logging.info("Number of found Playground examples: %s", len(examples))
- examples = list(filter(lambda example: example.tag.multifile is False,
examples))
- logging.info("Number of sinlge-file Playground examples: %s",
len(examples))
Review Comment:
This was duplicated elsewhere, right? For posterity, could you drop a link
to that location?
##########
playground/infrastructure/datastore_client.py:
##########
@@ -258,7 +265,7 @@ def _to_snippet_entity(
"pipeOpts": self._get_pipeline_options(example),
"created": now,
"origin": origin,
- "numberOfFiles": 1,
+ "numberOfFiles": 1 + len(example.tag.files),
Review Comment:
Why are we adding 1? Which file is not represented in `example.tag.files`
--
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]
