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

Fix timeout on ajax_verify_external_connection #245

Merged
merged 8 commits into from
Mar 24, 2020

Conversation

madmax3365
Copy link
Contributor

Related to #244. We can use these methods to check connections, it gives us a 4x performance boost.

@jeffpaul
Copy link
Member

jeffpaul commented Apr 1, 2019

@dkotter does your work touch this area of the code? If not, then I'm inclined to see if we can resolve the build error and look to land this alongside the performance improvements you've opened for v1.5.0.

@dkotter
Copy link
Collaborator

dkotter commented Apr 9, 2019

@jeffpaul Nope, the performance changes I've been working on have only touched how we get internal connections, not external.

@jeffpaul jeffpaul added this to the 1.5.0 milestone Apr 9, 2019
@jeffpaul jeffpaul added the type:enhancement New feature or request. label Apr 9, 2019
@jeffpaul
Copy link
Member

jeffpaul commented Jun 5, 2019

@madmax3365 it looks like the build failed on these changes, mind giving those a review to try and ensure this build passes?

@jeffpaul jeffpaul modified the milestones: 1.5.0, Future Release Jun 5, 2019
@jeffpaul jeffpaul added the needs:feedback This requires reporter feedback to better understand the request. label Jun 5, 2019
@madmax3365
Copy link
Contributor Author

@jeffpaul It seems that problem is in Travis. We have another pull request #262 that failed due to the same reason
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
I think you had some problems with Travis back then, so to avoid unnecessarily commits you can rerun the build.

@jeffpaul jeffpaul requested a review from dkotter June 6, 2019 18:34
@jeffpaul jeffpaul removed the needs:feedback This requires reporter feedback to better understand the request. label Jun 6, 2019
@jeffpaul jeffpaul modified the milestones: Future Release, 1.5.0 Jun 6, 2019
@jeffpaul
Copy link
Member

jeffpaul commented Jun 6, 2019

@adamsilverstein @dkotter if either of you have time to review this that would be great, thanks!

@helen
Copy link
Contributor

helen commented Jul 17, 2019

At first glance this makes a lot of sense to me - I'd like to group this together with #45 and #368 (not dependent on #368) so we can also use this to display a more granular status check for a given connection. Moving to 1.6.

@helen helen modified the milestones: 1.5.0, 1.6.0 Jul 17, 2019
Copy link
Contributor

@dinhtungdu dinhtungdu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with 100 CPT and this PR handle it very well while couldn't do the same with the current develop branch. Great work! Here are just some suggestions to make the codebase more maintainable.

includes/rest-api.php Outdated Show resolved Hide resolved
includes/rest-api.php Outdated Show resolved Hide resolved
@madmax3365
Copy link
Contributor Author

@dinhtungdu Applied code changes as you suggested.

Copy link
Contributor

@dinhtungdu dinhtungdu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@madmax3365 Thanks! I added some comments on your updates :)

@jeffpaul jeffpaul merged commit 6c5ac6b into 10up:develop Mar 24, 2020
@dinhtungdu dinhtungdu mentioned this pull request Mar 27, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New feature or request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants