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

switch to our regular createDataverseRequest pattern #8992 #8993

Merged
merged 1 commit into from
Sep 23, 2022

Conversation

pdurbin
Copy link
Member

@pdurbin pdurbin commented Sep 23, 2022

What this PR does / why we need it:

put("/api/edit/" + fileId) is failing

Which issue(s) this PR closes**:

Closes #8992

Special notes for your reviewer:

I'm putting in our usual pattern for getting a request. I'm not sure why this fixes it on my machine, honestly, but it does. Hopefully Jenkins agrees. The test was failing very consistently for me on my laptop.

@qqmyers suggested it might be a timing issue and I agree it's possible. I didn't add a sleep.

I'm not sure what changed. No recent PRs that were merged seem to have to do with editing DDI.

Suggestions on how to test this:

Make sure tests pass.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

No.

Is there a release notes update needed for this change?:

No.

Additional documentation:

None.

@pdurbin pdurbin added this to the 5.12 milestone Sep 23, 2022
@qqmyers
Copy link
Member

qqmyers commented Sep 23, 2022

Hmm - very odd. Before the PR the @context HttpServeletRequest in EditDDI was essentially overriding the one it inherited from AbstractAPIBean - perhaps the latest Payara handles that differently? In any case, assuming the fix works (tests aren't done yet), I'm not sure we have to know the details (although it would be nice if we can confirm no problem on an earlier payara, same code.)

@pdurbin
Copy link
Member Author

pdurbin commented Sep 23, 2022

perhaps the latest Payara handles that differently?

Not sure. When we upgraded Paraya all the tests passed: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-8949/8/testReport/edu.harvard.iq.dataverse.api/ That was in #8949.

Very strange.

@pdurbin
Copy link
Member Author

pdurbin commented Sep 23, 2022

@qqmyers suggested removing just the following...

-    @Context
-    protected HttpServletRequest httpRequest;

... which I did locally. I ran the test three times and it passes. That said, @landreev @qqmyers and I all agree that refactoring the method is fine to leave in this PR.

In terms of what changed, perhaps it was the merging of gdcc/dataverse-ansible#256 that started causing the test to fail? That was the Payara upgrade. Not sure why it would affect httpRequest. The reason I'm able to remove it is that it's already defined in the parent class, AbstractApiBean.

@pdurbin
Copy link
Member Author

pdurbin commented Sep 23, 2022

Oh, and Jenkins is happy with this PR. All tests passed.

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.

NPE when calling put("/api/edit/" + fileId)
2 participants