alexeyinkin commented on code in PR #24402:
URL: https://github.com/apache/beam/pull/24402#discussion_r1035676323
##########
playground/infrastructure/cd_helper.py:
##########
@@ -86,7 +86,7 @@ async def _populate_fields(example: Example):
if example.sdk in [SDK_JAVA, SDK_PYTHON]:
example.graph = await
client.get_graph(example.pipeline_id, example.filepath)
except Exception as e:
- logging.error(example.link)
+ logging.error(example.url_vcs)
Review Comment:
`vcs_url`
##########
playground/infrastructure/config.py:
##########
@@ -59,7 +59,7 @@ class Config:
CI_STEP_NAME = "CI"
CD_STEP_NAME = "CD"
CI_CD_LITERAL = Literal["CI", "CD"]
- LINK_PREFIX = "https://github.com/apache/beam/blob/master";
+ URL_VCS_PREFIX = "https://github.com/apache/beam/blob/master";
Review Comment:
Please change the order of words to 'VCS URL' here and everywhere else. Also
'notebook URL'.
This is a better English, and also Robert Martin suggests the most important
words at the end, and 'URL' is definitive among the two.
##########
playground/infrastructure/test_ci_helper.py:
##########
@@ -62,7 +62,7 @@ async def test__verify_examples():
output="output_of_example",
status=STATUS_FINISHED,
tag=Tag(**object_meta_def_ex),
- link="link")
+ url_vcs="link")
Review Comment:
A good opportunity to add trailing commas in this and other affected files.
##########
playground/infrastructure/helper.py:
##########
@@ -48,9 +49,10 @@
TagFields.pipeline_options,
TagFields.default_example,
TagFields.context_line,
- TagFields.tags
+ TagFields.tags,
+ TagFields.url_notebook,
],
- defaults=(None, None, None, False, None, None, False, None, None))
+ defaults=(None, None, None, False, None, None, False, None, None, None))
Review Comment:
😱
##########
playground/infrastructure/datastore_client.py:
##########
@@ -271,10 +270,12 @@ def _to_example_entity(self,
"descr": example.tag.description,
"tags": example.tag.tags,
"cats": example.tag.categories,
- "path": example.link,
+ "path": example.url_vcs, # keep for backward-compatibity, to
be removed
"type": PrecompiledObjectType.Name(example.type),
"origin": origin,
- "schVer": schema_key
+ "schVer": schema_key,
+ "urlVCS": example.url_vcs,
+ "urlNotebook": example.url_notebook,
Review Comment:
Can you check the Python's habits for acronyms? In Dart and many other
languages the habit is to capitalize only the first letter. For instance the
class for URI is named `Uri` in Dart. I guess `urlVcs` would better in this
case.
And also since we change to 'VCS URL' order this becomes `vcsUrl` and
`notebookUrl`.
##########
playground/infrastructure/test_helper.py:
##########
@@ -370,8 +384,8 @@ def test_validate_example_fields_when_code_is_invalid():
def test_validate_example_fields_when_link_is_invalid():
example = _create_example("MOCK_NAME")
- example.link = ""
- with pytest.raises(ValidationException, match="Example doesn't have a link
field. Path: MOCK_FILEPATH"):
+ example.url_vcs = ""
+ with pytest.raises(ValidationException, match="Example doesn't have a
github_url field. Path: MOCK_FILEPATH"):
Review Comment:
Is it `github_url` or `vcs_url`?
##########
playground/infrastructure/config.py:
##########
@@ -59,7 +59,7 @@ class Config:
CI_STEP_NAME = "CI"
CD_STEP_NAME = "CD"
CI_CD_LITERAL = Literal["CI", "CD"]
- LINK_PREFIX = "https://github.com/apache/beam/blob/master";
+ URL_VCS_PREFIX = "https://github.com/apache/beam/blob/master";
Review Comment:
I also suggest
```python
NOTEBOOK_URL_PREFIX =
"https://colab.research.google.com/github/apache/beam/blob/master";
```
--
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]
