Skip to content

Conversation

@stephenplusplus
Copy link
Contributor

Fixes #95

@stephenplusplus stephenplusplus requested a review from bcoe November 22, 2019 20:04
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 22, 2019
@codecov
Copy link

codecov bot commented Nov 22, 2019

Codecov Report

Merging #544 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #544   +/-   ##
=======================================
  Coverage   94.07%   94.07%           
=======================================
  Files          12       12           
  Lines         928      928           
  Branches      192      192           
=======================================
  Hits          873      873           
  Misses         44       44           
  Partials       11       11

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ad9b94...cb3881d. Read the comment docs.

Copy link

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

This is looking good, a couple small nits.

body: |-
### Troubleshooting
#### Emulator returning `DEADLINE_EXCEEDED`, `java.lang.OutOfMemoryError`
*Reference Issue: https://github.com/googleapis/nodejs-datastore/issues/95*
Copy link

Choose a reason for hiding this comment

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

it might be nice to make this a link:

*Reference Issue: [#95](https://github.com/googleapis/nodejs-datastore/issues/95)*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean make the text “Reference Issue” a link rather than the issue number? This is a weird consistency micro-concern of an edge case where we have multiple reference links.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh man, did my eyes trick me on this. Sorry, I'll make that change. What a goofball.

Copy link

Choose a reason for hiding this comment

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

I wasn't sure if GitHub linkifies links in READMEs, so was just thinking it's nice to have something clickable, we can confirm either way when we generate the README.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sorry, I thought you pasted the line the way I had it already, but you were actually writing out your suggestion. It was such a good suggestion, that I assumed I had already done it! 🤣

*Reference Issue: https://github.com/googleapis/nodejs-datastore/issues/95*
When using the emulator, you may experience errors such as "DEADLINE_EXCEEDED" within your application, corresponding to an error in the emulator: "java.lang.OutOfMemoryError". These errors are unique to the emulator environment and will not persist in production.
A workaround is available, provided by [@ohmpatel1997](https://github.com/ohmpatel1997) [here](https://github.com/googleapis/nodejs-datastore/issues/95#issuecomment-554387312).
Copy link

Choose a reason for hiding this comment

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

I'd be tempted to include the workaround here, explaining that you simply run Node.js with more memory; but with a link to the issue at the end of the line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was tempted as well, although I didn't actually understand how to pass those arguments to the emulator. I figured the comment would make sense to emulator users, so I just linked. But if you know the full command to pass those, I will pop it in here!

README.md Outdated
```
### Troubleshooting
#### Emulator returning `DEADLINE_EXCEEDED`, `java.lang.OutOfMemoryError`
*Reference Issue: [#95](https://github.com/googleapis/nodejs-datastore/issues/95)**
Copy link

Choose a reason for hiding this comment

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

need an extra * here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually removed one, I was just going for italics.

body: |-
### Troubleshooting
#### Emulator returning `DEADLINE_EXCEEDED`, `java.lang.OutOfMemoryError`
*Reference Issue: [#95](https://github.com/googleapis/nodejs-datastore/issues/95)**
Copy link

Choose a reason for hiding this comment

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

need an extra * here 👍

@stephenplusplus stephenplusplus merged commit 7a11494 into googleapis:master Nov 26, 2019
@stephenplusplus stephenplusplus deleted the spp--95 branch November 26, 2019 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Errors importing lots of entities to DataStore using the emulator

3 participants