search

getodk / briefcase

ODK Briefcase is a Java application for fetching and pushing forms and their contents. It helps make billions of data points from ODK portable. Contribute and make the world a better place! ✨💼✨
https://docs.getodk.org/briefcase-intro
Other
60 stars 156 forks source link

Make HTTP POST requests repeatable #856

Closed ggalmazor closed 4 years ago

ggalmazor commented 4 years ago

Make HTTP post requests repeatable by wrapping entities into a repeatable entity

In order to test this PR with Ona servers, https://stage-odk.ona.io must be used because it looks like the production servers haven't been patched yet.

Closes #852

What has been done to verify that this works as intended?

@DavisRayM has tested that this PR fixes the issues he's experiencing while pushing forms to Ona.

Why is this the best possible solution? Were any other approaches considered?

The changes in this PR come straight up from Apache HttpClient's documentation & examples, so no alternatives have been considered.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

The change is super narrow, so I don't expect any nasty side-effects, although manual tests should be extended to Aggregate and Central servers just to be sure nothing broke.

Does this change require updates to documentation? If so, please file an issue at https://github.com/opendatakit/docs/issues/new and include the link below.

Nope.

ggalmazor commented 4 years ago

Since Davis has already tested it and we have a hard deadline on Friday, I'm expediting this PR into the needs testing category

Hoping y'all agree with me ;)

lognaturel commented 4 years ago

Works for me! @kkrawczyk123 is still busy with Collect regression testing but hopefully she can take an hour or so to check basic pushes to Central and Aggregate and see if she thinks of anything else that should be verified.

ggalmazor commented 4 years ago

@DavisRayM, am I correct saying that the production servers don't have the 401 empty content patch yet? We're going to use the stage-odk instance to test this PR. Is that OK?

DavisRayM commented 4 years ago

@DavisRayM, am I correct saying that the production servers don't have the 401 empty content patch yet? We're going to use the stage-odk instance to test this PR. Is that OK?

Yes, you're correct @ggalmazor . I'll deploy the empty content fix to stage(currently not there)

DavisRayM commented 4 years ago

stage-odk.ona.io should be fine to use now 😄 @ggalmazor

kkrawczyk123 commented 4 years ago

@DavisRayM I am testing the pr, Did you use any specific form when you have encountered that issue? I am having some fails when pushing to ona server different forms - there might be some limitations ex. form versioning, specifying kind of media etc that do not occur for Central and Aggregate and you are aware of them but I am not.

DavisRayM commented 4 years ago

@DavisRayM I am testing the pr, Did you use any specific form when you have encountered that issue? I am having some fails when pushing to ona server different forms - there might be some limitations ex. form versioning, specifying kind of media etc that do not occur for Central and Aggregate and you are aware of them but I am not.

I didn't use any specific form while testing this.... Though @ggalmazor did report 404 errors popping up, these errors pop up in Ona due to the form version specified in the submission data being different from the version stored in Ona.

I might have answered this in a roundabout way ? Please let me know if this is unclear

In order to stop this from happening you might want to import data for forms that don't yet exist in your Ona stage account.

Making sure the version is the same on Ona and the submission data also helps.

kkrawczyk123 commented 4 years ago

@DavisRayM Are you able to push any form with media files to ona server?

DavisRayM commented 4 years ago

@kkrawczyk123 No not able to do so. Testing this out on different version of briefcase Don't want to block this PR if it's an Ona issue

DavisRayM commented 4 years ago

The 404 error seems to be consistent on the stage server even on the last stable version (atleast against Ona) v1.15.0 I'm assuming this is an issue on Ona's side. Most likely something to do with the version in the submission, sadly I can't research this at the moment...

I'd suggest not using Ona as the benchmark for now....

kkrawczyk123 commented 4 years ago

@ggalmazor @lognaturel as mentioned above I have encountered some issues when pushing forms to ona. They are connected to ona limitations like: ( forms/submissions with media are failing, form version differences forms with audits - I was not investigating all/other) not to briefcase and pr. I was able to push some forms to ona so I confirmed that it works for simple forms. I have also try pushing to Aggregate and Centrral all the ssme forms as for ona (also failing ones) and I haven't noticed any regression. I've checked cmd push too and it worked fine. If you have any other ideas what else should be verified please let me know. I was testing Ubuntu only.

lognaturel commented 4 years ago

Thanks, Kasia! That seems sufficient to me.

kkrawczyk123 commented 4 years ago

@opendatakit-bot label "behavior verified"