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

Fully launch 3p-use-ampcontext, and clean up old code #9307

Closed
3 of 4 tasks
lannka opened this issue May 12, 2017 · 8 comments
Closed
3 of 4 tasks

Fully launch 3p-use-ampcontext, and clean up old code #9307

lannka opened this issue May 12, 2017 · 8 comments

Comments

@lannka
Copy link
Contributor

lannka commented May 12, 2017

This is the follow up work for #8087

Proposed phases:

  • write a simple integration test checks the existence of all documented APIs
  • launch in 1% canary, check error logs
  • full launch
  • clean up code

Since this should be a no behavior change refactoring, it's optional for developers to opt-in and test. @jasti what you think? any channel to bring this up to developer's attention?

@jasti
Copy link
Contributor

jasti commented May 15, 2017

@lannka approach seems fine. I suggest we get to the point of launching 1% canary and notify developers in the AMP ads "team" in GH and Slack.

@jasti jasti added this to Feature Backlog in AMP Advertising Jun 1, 2017
@jasti jasti moved this from Feature Backlog to Current Sprint in AMP Advertising Jun 2, 2017
@jasti
Copy link
Contributor

jasti commented Jun 2, 2017

@alanorozco Is this going to finished today or does it need to be moved to the next sprint?

@jasti jasti moved this from Current Sprint to Bug Backlog in AMP Advertising Jun 15, 2017
@jasti jasti moved this from Bug Backlog to Current Sprint in AMP Advertising Jun 15, 2017
@jasti jasti modified the milestones: Sprint H1 June, Backlog Bugs Jun 15, 2017
@jasti jasti moved this from Current Sprint to Sprint Candidate in AMP Advertising Jun 16, 2017
@jasti jasti moved this from Sprint Candidate to Current Sprint in AMP Advertising Jun 20, 2017
@jasti jasti modified the milestones: Sprint H2 June, Sprint H1 June Jun 20, 2017
@lannka lannka modified the milestones: Fixit Week EOQ2, Sprint H2 June Jul 6, 2017
@lannka
Copy link
Contributor Author

lannka commented Jul 11, 2017

ix experienced an outage after this launching. we under looked the risks and meanwhile our communication wasn't enough to capture such a bug in testing stage.

To prevent this from happening again, we should have a more formal process for launching any changes might potentially affect 3rd parties.

@jasti what you say?
/cc @alanorozco

@lannka lannka modified the milestones: Backlog Bugs, Fixit Week EOQ2 Aug 7, 2017
@lannka lannka moved this from Current Sprint to Bug Backlog in AMP Advertising Aug 7, 2017
@lannka
Copy link
Contributor Author

lannka commented Jan 2, 2018

Tests are added and all issues are addressed.
See #10575 #10628 #10456 #8008 #12333

I'd like to move forward the launch again by starting a communication with all integrated ad networks.

No changes were made to the externally documented APIs. But we do suggest all ad networks to play with our new implementation by turning on the experiment feature flag.

  • Go to AMP experiment console page, and toggle the item 3p-use-ampcontext.
  • Go to any AMP pages that are integrated with your ad network, and see if everything is working as before. Make sure you tests ad slots that are relying on the context APIs.

However, if your integration relies on the old implementation, e.g. interacting with the APIs using postMessage composed by your code, this launch is a breaking change. Because we changed the postMessage format (to be unified across the whole AMP project), and we don't expect any ad network to really use that. If you're already doing so, you will need to make a change your side to get it fixed. Again, you can follow the above testing instruction to verify.

@jasti could you please forward this message to appropriate working group? The aimed timeline is to launch this 3 weeks later, which will be the 1/23/2018. (Let me know if you have any other suggestion about the timeline.)

@ampproject ampproject deleted a comment from ampprojectbot Jan 2, 2018
@lannka lannka assigned lannka and unassigned alanorozco and lannka Jan 2, 2018
@jasti
Copy link
Contributor

jasti commented Jan 2, 2018

@chrisdrexlerlemire-indexexchange @yieldmo-rao @ampproject/ads would you please be able to test the documented APIs before we launch?
An email has also been sent to amp-ads-announce@googlegroups.com group to communicate this change.

@ghost
Copy link

ghost commented Jan 5, 2018

Hi @jasti - we'll run some tests and let you know of the results. Appreciate the heads up!

@harquail
Copy link
Member

@jasti Yieldmo didn't see any impact of this change on our AMP implementation.
Thanks for letting us know!

@lannka
Copy link
Contributor Author

lannka commented May 9, 2018

Legacy code has been cleaned up: #14787

@lannka lannka closed this as completed May 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
AMP Advertising
  
Bug Backlog
Development

No branches or pull requests

5 participants