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

Removed XFormServerModifiedParams #28583

Closed
wants to merge 1 commit into from
Closed

Conversation

orangejenny
Copy link
Contributor

@snopoke @sravfeyn This looks like it exists to support this query. Would it be safe to merge this after dimagi/commcare-export#159 or do we need to keep it for the sake of backwards compatibility?

@orangejenny orangejenny added Open for review: do not merge A work in progress product/invisible Change has no end-user visible impact labels Oct 3, 2020
@sravfeyn
Copy link
Member

sravfeyn commented Oct 5, 2020

Hey @orangejenny Yes, I remember that's mostly from DTE. But you could do a quick grep on nginx logs to be 100%sure.

@snopoke
Copy link
Contributor

snopoke commented Oct 5, 2020

I expect we'd need to keep it for some time since it will take a while for all clients to be upgraded

@snopoke
Copy link
Contributor

snopoke commented Oct 5, 2020

When we do remove it we should add an error message as well so that DET users know what to do to fix it.

@orangejenny
Copy link
Contributor Author

@sravfeyn @snopoke Sounds good, thanks. I'll leave this open and revisit in a while.

@snopoke
Copy link
Contributor

snopoke commented Mar 31, 2023

A quick data pull of our logs shows this query is still being used by a number of domains. I see commcare-export versions 1.8.1 (latest), 1.4.0, 1.2.1. I think removing this will require forcing upgrades and re-syncing of data which may not be tractable for some large domains.

I think we could come up with a way to seamlessly migrate to the new pagination style but we'll still need to force an upgrade of commcare-export. I've made an issue for that here: dimagi/commcare-export#220

@orangejenny
Copy link
Contributor Author

@snopoke Realistically I'm unlikely to take this forward. Would it make sense to go in USH's tech debt backlog?

@snopoke
Copy link
Contributor

snopoke commented Apr 3, 2023

@snopoke Realistically I'm unlikely to take this forward. Would it make sense to go in USH's tech debt backlog?

I think we can just punt it. I've created the issue in the commcare-export repo so it won't get lost.

@snopoke snopoke closed this Apr 3, 2023
@snopoke snopoke deleted the jls/indexed_on-cruft branch April 3, 2023 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Open for review: do not merge A work in progress product/invisible Change has no end-user visible impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants