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

History beta poll decay time reset #11178

Merged
merged 3 commits into from
Jan 21, 2021

Conversation

Nerdinacan
Copy link
Contributor

@Nerdinacan Nerdinacan commented Jan 19, 2021

The beta history poll slows down over time so we're not asking the server for useless things during periods of inactivity, but it makes sense to speed up whenever we start manipulating content.

I've modified how the polling works to include an observable that looks at the browser XHR object and emits an event when a matching method / url are detected, this emission is used to reset the decay time on the polling.

@Nerdinacan Nerdinacan self-assigned this Jan 19, 2021
@github-actions github-actions bot added this to the 21.05 milestone Jan 19, 2021
@Nerdinacan Nerdinacan marked this pull request as draft January 19, 2021 19:12
@Nerdinacan Nerdinacan marked this pull request as ready for review January 20, 2021 05:18
@dannon
Copy link
Member

dannon commented Jan 20, 2021

This doesn't seem to be working like I'd expect. I added debugging to the route matcher and that's firing (ROUTE MATCHED, SHOULD RESET), but it isn't resetting the decay in my hands, note the timestamps:

image

The other cruft you see logged is just a tap() I was debugging with.

@Nerdinacan
Copy link
Contributor Author

Thanks man, I'll beat on it some more. Was trying to have a global solution that didn't involve us touching every outgoing ajax POST.

@Nerdinacan Nerdinacan marked this pull request as draft January 20, 2021 18:46
@dannon
Copy link
Member

dannon commented Jan 20, 2021

@Nerdinacan No worries at all -- I really like the approach, and the watching is all working great. We just need to get the restoration to $freshPoll or otherwise resetting decay working right.

@Nerdinacan
Copy link
Contributor Author

Nerdinacan commented Jan 20, 2021

@dannon

Yep got it. I think it was just misconfigured. I left a debug line in decay() so you can see it reset in the console.

On a side note I really wish there was a way to make prettier understand piped functions. I hate having to add prettier-ignore because prettier legitimately finds a lot of dumb formatting bugs. But it leaves every observable stream unintelligible.

@Nerdinacan Nerdinacan marked this pull request as ready for review January 20, 2021 20:00
@dannon
Copy link
Member

dannon commented Jan 20, 2021

@Nerdinacan I'll see what I can figure out. Agree it'd be great to just let prettier process these files (and notice issues), but putting all the piped stuff in a single line does indeed suck.

Dropped a couple prettier-ignores that weren't doing much, but we
can/should certainly continue to use this where the formatting is
egregious.
@dannon
Copy link
Member

dannon commented Jan 21, 2021

@Nerdinacan Thanks. This is working great now; I pushed a commit with minor cleanup and a typo fix but no functional changes.

I some digging into making prettier work better with RxJS in general, and it's an issue folks are aware of there. There have been a few attempts at it, and maybe they'll get it right at some point, but for now I'd just say scope // prettier-ignore to the most local block you need and just do it that way.

(for what it's worth, part of the problem here is actually our wide 120 character code width -- most of the rxjs is actually pretty decent to read when wrapped to 80)

@dannon dannon merged commit 5b0f47f into galaxyproject:dev Jan 21, 2021
@Nerdinacan Nerdinacan deleted the prod_polling_decay branch January 21, 2021 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants