New class to handle async XML-RPC requests #16674
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
Thank you for the great PR description! When this PR is ready for review, please apply the E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16674 Scheduled Jetpack release: September 1, 2020. |
leogermani
changed the title
leogermani
added
[Status] Needs Review
leogermani
force-pushed
the
add/xmlrpc-async-call-lib
branch
3 times, most recently
from
e6c2051 to
455225a
Compare
kraftbj
previously requested changes
Aug 3, 2020
Contributor
kraftbj
left a comment
kraftbj
left a comment
There was a problem hiding this comment.
Quick read through the code reads okay. I didn't test, but wonder if having it as part of the connection package is long-term more functional?
leogermani
added
[Status] Needs Author Reply
jeherve
added
[Type] Enhancement
leogermani
force-pushed
the
add/xmlrpc-async-call-lib
branch
from
56c669e to
8afe88b
Compare
github-actions
bot
added
the
[Status] Needs Package Release
leogermani
added
[Status] Needs Review
Contributor
Author
|
Moved into the connection package |
Combining the `multisite` and `single-site` functions into one to improve readability.
sergeymitr
approved these changes
Aug 4, 2020
Contributor
sergeymitr
left a comment
sergeymitr
left a comment
There was a problem hiding this comment.
@leogermani I pushed a commit to simplify the code a little bit, hope that's ok.
Tested it on both single-site and multisite installs, works as a charm 👍 
sergeymitr
added
[Status] Ready to Merge
Contributor
Author
|
Thanks @sergeymitr ! Pinging @kraftbj to see if he want to take a look after the changes |
kraftbj
approved these changes
Aug 5, 2020
matticbot
added
[Status] Needs Changelog
and removed
[Status] Ready to Merge
davidlonjon
added a commit
that referenced
this pull request
Aug 6, 2020
…ic/jetpack into add/jetpack-lazy-images-package * 'add/jetpack-lazy-images-package' of github.com:Automattic/jetpack: (40 commits) Lodash: Revert to previous version (#16735) New class to handle async XML-RPC requests (#16674) Social Previews: Sidebar design updates (#16725) Update E2E locator for classic connection flow (#16708) Woo Services: update to use existing Jetpack plugin install tools (#16672) Admin Page: avoid blank dashboard when notice is not a string (#16721) Admin Page: update string still using old i18n format (#16722) Social Previews: Add sidebar UI (#16633) Fix recurring payments block disconnecting (sometimes) when existing article is reopened in block editor. (#16640) Connection Register: Add current user email to connection register request (#16712) Update versions to start 8.9 Release cycle (#16706) Donations block: Make currency and amounts editable (#16593) Update dependency @automattic/calypso-color-schemes to v2 Error Notice: removing HTML code, adjusting maximum width. (#16690) Update dependency swiper to v6 (#16587) Site Scan: Short-circuit update_threats_link() if Admin Bar is not present (#16677) Update vulnerable NPM packages (#16659) E2E Tests: Add Jetpack updater test (#16437) check for subdir site before rendering Ads.txt section (#16671) VideoPress Block: Retain alignment support (#16651) ...
pereirinha
pushed a commit
that referenced
this pull request
Sep 10, 2020
* add async call class to connection package * add docs to the new class * Async XMLRPC Requests: Simplifying the code. Combining the `multisite` and `single-site` functions into one to improve readability. Co-authored-by: Sergey Mitroshin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduces a new class to handle async XML-RPC requests and replace
Jetpack::xmlrpc_async_call()Changes proposed in this Pull Request:
Currently,
Jetpack::xmlrpc_async_call()is used to enqueue few XML-RPC requests that are sent all at once, as a MultiCall request, at shutdown. This method forces the requests to use the "Master user" token.As we are moving to give preference to the blog token everywhere we can, and the current user token where we are performing a user action, we needed a more flexible method that allowed us to choose which token we wanted to use.
To modify this method while keeping backward compatibility would result in a not very elegant solution, polluting the Jetpack class even more and adding complexity to an untested method. So I decided to create a new class to handle this, removing the responsibility from the
Jetpackclass and start writing simple tests for it.This PR only adds the class but don't use it yet. Following PRs will replace the use of the deprecated method in favor of this class.
Note: I've first added a deprecation notice to the old method, but it breaks the tests, so I will first replace all the calls to it, and then we can review this PR to add the notice.
Jetpack product discussion
Does this pull request change what data or activity we track or use?
No
Testing instructions:
Proposed changelog entry for your changes: