Skip to content

Site: Endpoint UI, make the request wpcom only and only return plan data#6249

Merged
enejb merged 1 commit intomasterfrom
update/imrpove-site-endpoint-performance
Mar 8, 2017
Merged

Site: Endpoint UI, make the request wpcom only and only return plan data#6249
enejb merged 1 commit intomasterfrom
update/imrpove-site-endpoint-performance

Conversation

@enejb
Copy link
Copy Markdown
Member

@enejb enejb commented Feb 1, 2017

Make the request agains the shadow site.

Changes proposed in this Pull Request:

Testing instructions:

Does the plans page show up as expected?

@enejb enejb requested a review from eliorivero February 1, 2017 20:12
@enejb enejb self-assigned this Feb 1, 2017
@enejb enejb added [Status] Needs Review This PR is ready for review. [Team] Poseidon labels Feb 1, 2017
@enejb enejb requested review from gravityrail and lezama February 1, 2017 22:47
@enejb
Copy link
Copy Markdown
Member Author

enejb commented Feb 1, 2017 •
edited
Loading

Other improvements I think we can make is

  • Only request the necessary fields. I think in this case it is just the plan field.
  • Store the value of the api in a transient, but we need to make sure we are able to bust the cache. Since we don't want to introduce errors where we show the user the wrong plan after they just updated it.

@jeherve jeherve added this to the 4.7.0 - March 2017 milestone Feb 2, 2017
@jeherve jeherve added the Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Feb 2, 2017
@samhotchkiss samhotchkiss removed this from the 4.7.0 - March 2017 milestone Feb 3, 2017
@dereksmart
Copy link
Copy Markdown
Contributor

dereksmart commented Feb 10, 2017

@enejb what does this change do?

Watching the network console, I'm seeing that the response times for both this and master are the same. I also noticed that the response json is the same as well. What does removing '?force=wpcom' do?

@enejb
Copy link
Copy Markdown
Member Author

enejb commented Feb 10, 2017

It grabs the data form the .com's shadow site instead of the going back to the Jetpack site to generate the data

force=wpcom = jetpack Site -> wpcom -> data returns
force=jetpack (default) jetapck Site -> wpcom -> jetpack site -> wpcom -> data retruns

How are you testing the change? And timing it?

@enejb
Copy link
Copy Markdown
Member Author

enejb commented Feb 10, 2017

This is what I am seeing
screen_shot_2017-02-10_at_12_12_23

@dereksmart
Copy link
Copy Markdown
Contributor

dereksmart commented Feb 13, 2017

@enejb

How are you testing the change? And timing it?

I was watching the /site request/response in the Network console.
screen shot 2017-02-13 at 9 35 01 am

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Feature] WPCOM API and removed [Status] Needs Review This PR is ready for review. labels Feb 13, 2017
@enejb enejb added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 2, 2017
@enejb
Copy link
Copy Markdown
Member Author

enejb commented Mar 2, 2017

I think what might be happening is that your are always retrieving the api from wordpress.com for your site. There is a setting in calypso that lets you toggle that.

When I did a test for force=jetpack vs force=wpcom
I notices about a second difference between the 2 requests.

force=wpcom
screen shot 2017-03-02 at 14 13 13

force=jetpack ( default and current setting for most sites )
screen shot 2017-03-02 at 14 17 55

lezama
lezama approved these changes Mar 8, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@lezama lezama left a comment

Choose a reason for hiding this comment

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

Ideally we should have a different endpoint for getting this.

@enejb enejb merged commit 4a9f4da into master Mar 8, 2017
@enejb enejb deleted the update/imrpove-site-endpoint-performance branch March 8, 2017 17:57
@matticbot matticbot removed the [Status] Needs Review This PR is ready for review. label Mar 8, 2017
jeherve added a commit that referenced this pull request Mar 9, 2017
@jeherve
Changelog: add #6249
481e4b4
jeherve added a commit that referenced this pull request Mar 28, 2017
@jeherve
Changelog: add #6249
33bab72
samhotchkiss pushed a commit that referenced this pull request Mar 29, 2017
@jeherve @samhotchkiss
Jetpack 4.8: changelog. (#6613)
0d49aa5 
* Readme: remove old release and add skeleton for 4.8.

* Changelog: add #6572

* Changelog: add #6567

* Changelog: add #6542

* Changelog: add #6527

* Changelog: add #6508

* Changelog: add #6478

* Changelog: add #6477

* Changelog: add #6249

* Update stable version and remove old version from readme.

* Changelog: add 4.7.1 to changelog.

* Readme: add new contributor.

* Sync: update docblock @SInCE version.

Related: #6053

* Changelog: add release post.

* changelog: add #6053

* Changelog: add #6413

* Changelog: add #6482

* Changelog: add #6584

* Changelog add #6603

* Changelog: add #6606

* Changelog: add #6611

* Changelog: add #6635

* Changelog: add #6639

* Changelog: add #6684

* Changelog: add #6710

* Changelog: add #6711

* Changelog: add #5461

* Testing list: update Settings UI feedback prompt.

Props @MichaelArestad

* Changelog: add #6789

* Changelog: add #6778

* Changelog: add #6777

* Changelog: add #6775

* Changelog: add #6755

* Changelog: add #6731

* Changelog: add #6721

* Changelog: add #6705

* Changelog: add #6702

* Changelog: add #6671

* Changelog: add #6637

* Changelog: add #6582

* Changelog: add #6566

* Changelog: add #6555

* Changelog: add #6529

* Changelog: add #6344

* Changelog: add #5763

* Changelog: add #5503

* Changelog: update #6637 changelog.

@see 40e115c#commitcomment-21523982

* Changelog: add #6699

* Changelog: add #6632

* Changelog: add #6769

* Changelog: add #6707

* Changelog: add #6590
zinigor pushed a commit that referenced this pull request Apr 19, 2017
@enejb @zinigor
JSON API: Site Endpoint add access to shadow site for blog token
a598613 
This change will allow Jetpack sites higher permission when accessing the site api endpoint and retrieve the data from the shadow site directly.

Code review in https://[private link]
Related #6249

This was will also fix the Jetpacks plan page for users that default to using the
Shadow site data.

Merges r150232-wpcom.
zinigor pushed a commit that referenced this pull request Apr 21, 2017
@enejb @zinigor
JSON API: Site Endpoint add access to shadow site for blog token
4744ecf 
This change will allow Jetpack sites higher permission when accessing the site api endpoint and retrieve the data from the shadow site directly.

Code review in https://[private link]
Related #6249

This was will also fix the Jetpacks plan page for users that default to using the
Shadow site data.

Merges r150232-wpcom.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lezama lezama lezama approved these changes

@eliorivero eliorivero Awaiting requested review from eliorivero

@gravityrail gravityrail Awaiting requested review from gravityrail

Assignees

@enejb enejb

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] WPCOM API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@enejb @dereksmart @lezama @jeherve @samhotchkiss @matticbot