Repost with deletions/edits v1 #3687
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
in ClaimReference.
…tputs(). Test modification of file_size, languages in a repost.
moodyjon
commented
Oct 13, 2022
| @@ -23,4 +23,4 @@ idea: | |||
| cp -r scripts/idea/* .idea | |||
|
|
|||
| elastic-docker: | |||
| docker run -d -v lbryhub:/usr/share/elasticsearch/data -p 9200:9200 -p 9300:9300 -e"ES_JAVA_OPTS=-Xms512m -Xmx512m" -e "discovery.type=single-node" docker.elastic.co/elasticsearch/elasticsearch:7.12.1 | |||
| docker run -d --env network.publish_host=127.0.0.1 -v lbryhub:/usr/share/elasticsearch/data -p 9200:9200 -p 9300:9300 -e"ES_JAVA_OPTS=-Xms512m -Xmx512m" -e "discovery.type=single-node" docker.elastic.co/elasticsearch/elasticsearch:7.12.1 | |||
There was a problem hiding this comment.
I seem to have to use this to get "make elastic-docker" to work. Is it just me? (MacOS aarch64)
moodyjon
commented
Oct 13, 2022
Comment on lines
+423
to
+430
| def update(self, **kwargs) -> dict: | ||
| claim_type = kwargs.pop('claim_type', None) | ||
| # Update common fields within BaseClaim. | ||
| kwargs = super().update(strict_update=False, **kwargs) | ||
| if claim_type: | ||
| # Remaining updates go into deletes/edits of ClaimReference. | ||
| kwargs = self.reference.update(claim_type, **kwargs) | ||
| return kwargs |
There was a problem hiding this comment.
See PR description. This is where I have a choice of applying the update to BaseClaim or stuff it inside ClaimReference.
|
gonna hold off on reviewing this for now. we discussed how edit/deletions are a bigger project that we need to tackle now, and we'll think about them again once the extensions work is done and we see how it plays in the wild |
|
Closed in favor of #3706 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: lbryio/types#49
Definitely very rough, with only a few attributes tested. Just putting this out there as an object for discussion.
The issue I'm currently wrestling with is how to treat common fields like tags, languages, title, description. When an update() request to set (or clear) such a field comes from
jsonrpc_stream_update(), do I put it inBaseClaim, orClaimReference? I am trying to set these first in theRepost(BaseClaim), then for the ones that fail, I try to put them inClaimReference(either deletions or edits).Maybe there should be separate args or a mode flag at the level of
jsonrpc_stream_updatethat selects where to apply the modifications? Regular mode would modify the Repost. Embedded mode would modify the ClaimReference (deletions/edits).I could also do something like split the updates into deletions and edits first. For deletions, try the BaseClaim (of the Repost) first, then fallback to ClaimReference. For edits, try the ClaimReference first, then fallback to BaseClaim (of the Repost).
Protobuf changes: