Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs update for unloadRecord #6181

Merged
merged 1 commit into from
Jun 24, 2019
Merged

Docs update for unloadRecord #6181

merged 1 commit into from
Jun 24, 2019

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Jun 23, 2019

The way this was phrase implies the exactly wrong thing. unloadRecord is definitely not about "destroying" anything.

The way this was phrase implies the exactly wrong thing. `unloadRecord` is *definitely not* about "destroying" anything.
@ef4 ef4 merged commit 93c46fe into master Jun 24, 2019
@ef4 ef4 deleted the unload-record-docs branch June 24, 2019 15:01
@runspired
Copy link
Contributor

Just noticed this. unloadRecord does destroy the record instance. You are correct that it does not send a DELETE request to the server, but it does (literally) call record.destroy(). The phrasing here is unfortunate because there are multiple levels of concepts and language semantics involved.

@runspired
Copy link
Contributor

We should either revert this or put in a new fix as the new documentation isn't accurate: unloadRecord does not unload any "requests" from memory, it clears as much data as it can from the cache and destroys the record instance. Any pending requests will still continue and once they resolve could resurrect the record and repopulate the cache.

@ef4
Copy link
Contributor Author

ef4 commented Jun 25, 2019

Ugh, yeah, that second "request" was a typo. I can do a followup PR.

ef4 added a commit that referenced this pull request Jun 26, 2019
pete-the-pete pushed a commit to pete-the-pete/data that referenced this pull request Jul 23, 2019
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.

3 participants