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

Fix unnecessary livestream api calls in channel page #6652

Merged
merged 3 commits into from Aug 1, 2021
Merged

Conversation

btzr-io
Copy link
Collaborator

@btzr-io btzr-io commented Jul 22, 2021

PR Checklist

Please check all that apply to this PR using "x":

  • I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
  • I added a line describing my change to CHANGELOG.md
  • I have checked that this PR does not introduce a breaking change
  • This PR introduces breaking changes and I have provided a detailed explanation below

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting)
  • Refactoring (no functional changes)
  • Documentation changes
  • Other - Please describe:

Fixes

Issue Number: #6632
Changelog update: #6735


React.useEffect(() => {
if (livestreamChannelId) {
// Don't search empty channels
if (livestreamChannelId && !isChannelEmpty) {
Lbry.claim_search({
Copy link
Member

Choose a reason for hiding this comment

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

Nice.
This might be faster with page_size=1 and no_totals?
@tzarebczan

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there any difference between claim_list and claim_search ?
https://lbry.tech/api/sdk#claim_list

Copy link
Member

Choose a reason for hiding this comment

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

claim list is specifically one's own publishes.

Copy link
Member

Choose a reason for hiding this comment

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

page=1 is a given - page_size=1 would limit results. Not sure how big of a deal it would be.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you publish something while livestreaming ?
I think will be safer to check more than one claim.

Copy link
Contributor

Choose a reason for hiding this comment

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

the "has no source param" ensures it's only the live streams that are returned. And right now we use the latest one you have published.

Copy link
Member

@jessopb jessopb left a comment

Choose a reason for hiding this comment

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

👍

@btzr-io
Copy link
Collaborator Author

btzr-io commented Jul 29, 2021

Changelog update: #6735

@btzr-io
Copy link
Collaborator Author

btzr-io commented Jul 30, 2021

Any reason for not merging this yet ?

@jessopb jessopb merged commit 37d9cbf into master Aug 1, 2021
@jessopb jessopb deleted the livestream-api branch August 1, 2021 13:13
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.

None yet

3 participants