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

Remove "Update Translators" #192

Closed
dstillman opened this issue Oct 4, 2017 · 7 comments
Closed

Remove "Update Translators" #192

dstillman opened this issue Oct 4, 2017 · 7 comments

Comments

@dstillman
Copy link
Member

It doesn't seem like we need both Update Translators and Reset Translators in the connectors. Can we remove the former?

@adomasven
Copy link
Member

adomasven commented Oct 4, 2017

We have both in the client and they do slightly different things, which is more relevant when Zotero is unavailable and translators are fetched from the repo. Although one could argue that with this safeguard in place resetting translators is not really relevant anymore.

@dstillman
Copy link
Member Author

Well, they do very different things in the client, they're presented very differently, and there's a specific reason for both, even under normal usage. Here, at least with the client open, there's no clear difference between the two, so we'd want to keep the more comprehensive one, which is Reset — the sanity check is a little imprecise, and it's important to have a way to make sure people have all translators when troubleshooting.

I hadn't thought about client-less usage, though. What's the actual difference between the two in client-less mode? I guess people do need a way to check for an updated translator without waiting, but how expensive is Reset, actually? (And if we do need both, Update should be moved to General and presented more like in the client rather than staying in Advanced.)

(Side note: we should probably consider more frequent metadata updates from the repo, to go along with our SSE/WebSockets mechanisms. We can optimize things server-side if necessary to make it more efficient.)

@adomasven
Copy link
Member

I hadn't thought about client-less usage, though. What's the actual difference between the two in client-less mode? I guess people do need a way to check for an updated translator without waiting, but how expensive is Reset, actually?

Reset fetches all translator metadata, update only checks for latest updates with some sort of timestamped mechanism and deletes all translator code, so it needs to be refetched for individual translators (which occurs on-the-fly as translator target regexp gets matched for detection).

Either way, I'd like to leave this decision to you and the implementation is nigh trivial.

(Side note: we should probably consider more frequent metadata updates from the repo, to go along with our SSE/WebSockets mechanisms. We can optimize things server-side if necessary to make it more efficient.)

I have code that updates connector translators upon client update via an SSE notification in some branch somewhere.

@dstillman
Copy link
Member Author

dstillman commented Oct 4, 2017

I'll take a look at how expensive a reset is server-side and go from there.

(Side note: we should probably consider more frequent metadata updates from the repo, to go along with our SSE/WebSockets mechanisms. We can optimize things server-side if necessary to make it more efficient.)

I have code that updates connector translators upon client update via an SSE notification in some branch somewhere.

Sorry, I meant from the z.org repo. As in, since we now have fast updates for the client via WebSockets and for the connector via SSE when used with the client, we should do more frequent updates for people using the connector alone.

@dstillman
Copy link
Member Author

@adomasven: Seems like we can definitely do this now that we have immediate updates from the app, right? And Reset would still cause an update if the app is unavailable?

@adomasven
Copy link
Member

The only use case for Update when Zotero is unavailable is when a fixed translator has been released, and the user just wants to fetch it from the repo. Reset will also accomplish that, but also remove all cached translator code from the Connector, which means it will hit the Repo for every translator where URL regex matches when navigating. In practice I can't imagine this is an issue, since e.g. restarting the browser also causes all cached code to be removed. Given that restart caching will be addressed with #465, users pressing Reset and refetching translators is going to becoming a relatively minor part of the load on the repo overall compared to before #465, and given how non-obvious it is for users whether to use Update Translators, or Reset Translators for their given scenario, I think it's appropriate to just remove it.

@dstillman
Copy link
Member Author

OK, makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants