Skip to content

[Browser] Catch name fetch errors for page rendering; Use V1 API to fetch properties#1846

Merged
shifucun merged 3 commits intodatacommonsorg:masterfrom
shifucun:appjs
Nov 18, 2022
Merged

[Browser] Catch name fetch errors for page rendering; Use V1 API to fetch properties#1846
shifucun merged 3 commits intodatacommonsorg:masterfrom
shifucun:appjs

Conversation

@shifucun
Copy link
Copy Markdown
Contributor

No description provided.

@shifucun shifucun requested a review from beets November 18, 2022 19:37
Copy link
Copy Markdown
Collaborator

@beets beets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for these fixes!

Comment thread server/routes/api/shared.py Outdated
result[dcid] = ''
if dcid in response:
values = response[dcid]
if values:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps: if values and len(values)? i'm slightly uncomfortable simply calling values[0]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread server/routes/browser.py Outdated
Comment on lines +36 to +38
node_name = shared_api.names([dcid]).get(dcid)
if not node_name:
node_name = dcid
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since node_name is already dcid by default, maybe instead:

try:
  api_name = shared_api.names...
  if api_name:
    node_name = api_name

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Copy Markdown
Contributor Author

@shifucun shifucun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for review!

Comment thread server/routes/api/shared.py Outdated
result[dcid] = ''
if dcid in response:
values = response[dcid]
if values:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread server/routes/browser.py Outdated
Comment on lines +36 to +38
node_name = shared_api.names([dcid]).get(dcid)
if not node_name:
node_name = dcid
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@shifucun shifucun merged commit 9870a82 into datacommonsorg:master Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants