Problem/Motivation

There doesn't seem to be a module integrating with Marketo's REST API: http://developers.marketo.com/documentation/rest/

This module supports both SOAP and Munchkin APIS, should it also support REST?

Proposed resolution

Discuss whether this would be best suited to this module or a separate stand alone module?

Remaining tasks

Discuss whether this would be best suited to this module or a separate stand alone module?

User interface changes

?

API changes

Addition of support for main features provided by Marketo's REST API

Data model changes

?

Comments

mccrodp created an issue. See original summary.

drupgirl’s picture

drupgirl commented

There's the sandbox module Marketo Forms 2.0 JS Embed Prefill that uses the REST API to prefill Marketo forms.

mccrodp’s picture

mccrodp commented

Thanks for this @drupgirl, it will likely come in useful if we decide to go ahead with providing an alternate REST API "Tracking Method".

jaydee1818’s picture

jaydee1818 commented

Hello, yes I am just looking into incorporating Marketo tracking on our site and I note that in the documentation found in the link below, the first thing Marketo do is suggest using the REST API ahead of the SOAP API.

http://docs.marketo.com/display/public/DOCS/Configuring+Your+SOAP+API+Se...

mccrodp’s picture

mccrodp commented

Great to hear @jaydee1818, let's collaborate: https://github.com/mccrodp/marketo_rest

ping me on IRC: mccrodp or email ;)

jyokum’s picture

jyokum commented

Hi all. I've been on hiatus for a while but finally getting back to contribute here. REST wasn't originally built into the module since Marketo didn't offer it at the time and its take it a while to mature to the point where it had enough equivalent functionality to make it worth the effort to build it in. I'll take a look at the recent Marketo REST APIs and the work you all have been doing so it can get included when ready.

mccrodp’s picture

mccrodp commented
Related issues: +#2610558: [marketo_ma] Marketo MA

Hey @jyokum. Welcome back and thanks for the reply.

When I started working with REST I considered a patch on marketo_ma, but when I received no reply I thought about a separate module also. For this reason and that Marketo recommends REST over SOAP and Marketo development of the SOAP API having stopped, this made sense to me. I also did not want to support SOAP and wasn't sure if you were still working with Drupal.

It seems obvious now that you certainly are still working on this and a patch may have made sense in this light to avoid duplication of effort. However, there have been quite a few module modifications since forking, including schema changes for the "Field Definitions" as REST has a different naming convention to SOAP / Munchkin API.

I know Jaesin has been working on D8 (see related issue) also as Marketo REST was not stable yet, so I'm not 100% sure what to do now. To proceed with Marketo REST and allow for separation and ease of phasing out SOAP API in future if that becomes the sensible option (SOAP API is certainly not deprecated at this point however).

It would be great to get your input on this anyway as I imagine Marketo REST will exist in the interim and since the majority of it was ported from Marketo MA, I could add you as maintainer if that was suitable. I'm concerned about duplication of effort though, so community feedback is appreciated here on what the best approach is, taking into account that both APIs are quite different in some cases and the amount of effort it would take to merge code at this point.

Thanks for your work on this module, it was a massive help to get started with REST integration.

Jaesin’s picture

Jaesin commented

In the D8 version we have been working on (https://github.com/Jaesin/marketo_ma), we are using my fork of the "Unofficial Marketo REST library" (https://github.com/jaesin/marketo-rest-api/). My fork adds a couple of the things we need for this module. The maintainer has been responsive via email but has not merged my pull request yet (https://github.com/dchesterton/marketo-rest-api/pull/34).

Jaesin’s picture

Jaesin commented

IMHO it would make since to use the `marketo_rest` module as the 7.x-2.x version of `marketo_ma`.

mccrodp’s picture

mccrodp commented

IMHO it would make since to use the `marketo_rest` module as the 7.x-2.x version of `marketo_ma`.

Thanks for your input Jaesin, I don't know why this hadn't crossed my mind. I think it's a fantastic idea and it was also seconded by Steve Persch when I was discussing the module with him.

Seeing as your work on D8 is developed primarily with REST, but could potentially work with SOAP by swapping out the service class to support another library, I think the 2.x version being REST/Munchkin and being under the marketo_ma namespace makes so much sense. It removes duplication of effort as much as possible and has the least work involved, keeping SOAP/Munchkin under 1.x so as to make deforking the marketo_rest module back under the marketo_ma namespace more trivial.

What do you think @jyokum, is this something we can work towards? It would be great to have all under the same roof and remove the marketo_rest project if so.

jyokum’s picture

jyokum commented

I think keeping it all in the existing marketo_ma project is the way to go and introducing REST in the 2.x version is reasonable. Couple of questions for the group to help clarify the motivation and goals associated with adding REST to the D7 version.

  • Is the goal to replace SOAP with REST as the underlying communication vehicle or to add REST as another option?
  • If the goal is to replace SOAP, what are the current deficiencies or limitations we are trying to address?
  • If the goal is to add REST as another option, what are the benefits or additional capabilities that become available?
  • As a module user, why would I choose one method vs. the other?
Jaesin’s picture

Jaesin commented

It looks like the REST and Soap APIs are both still supported but new features will be in the REST API only.

http://developers.marketo.com/blog/marketo-rest-vs-soap-apis-faq/

Some better performance with the REST API.
There aren't any limitations that I ran across in the D8 port as of yet.

For 7.x-2.x it might be better to just have REST. Both APIs being equal, It's a choice you don't have to burden site builders with.

mccrodp’s picture

mccrodp commented

Is the goal to replace SOAP with REST as the underlying communication vehicle or to add REST as another option?

I would imagine replacing SOAP as the default API would be the goal, but any future features added to 2.x (REST) that 1.x (SOAP) users really want could be possible also. i.e. - it's possible to use SOAP in future if there is demand, as in Jaesin's current state of the D8 version developers can swap the service class to use SOAP, in D7 new features could be backported via patch on 1.x if provided by the community.

I agree that removing the burden of API choice SOAP v.s REST is a good thing. Also, supporting REST only reduces module maintainability and is inline with Marketo's recommendation on using REST over SOAP. That is also a driving factor. http://docs.marketo.com/display/public/DOCS/Configuring+Your+SOAP+API+Se...

If the goal is to replace SOAP, what are the current deficiencies or limitations we are trying to address?

Supporting REST in 2.x is a step towards what Jaesin has in D8. No limitations with the current SOAP implementation, but any new features added to the REST API will likely not exist in SOAP.

If the goal is to add REST as another option, what are the benefits or additional capabilities that become available?

The APIs are actually quite different in some cases, so supporting both could have overhead. At the moment SOAP implementation is certainly more stable and as an example of an item remaining where I need extra time / help with is detailed in this issue: https://github.com/mccrodp/marketo_rest/issues/5 The REST equivalent at the moment for what we had in SOAP is not so suitable and perhaps needs rethinking. I believe these difference and inherent overhead could become more apparent as features are added in solution providing both as configurable options.

I feel adding the PHP client library as is employed in D8 and also detailed here is a benefit and could be a leap toward ease of adding new API features while being a step toward D8 : https://github.com/mccrodp/marketo_rest/issues/4

As a module user, why would I choose one method vs. the other?

Mainly being that the Marketo recommendation gives people more confidence in it. That and that D7 users looking toward D8 will see it is REST that is the standard for both exposing services and consuming via Guzzle library in core.

Giving module developers access to contribute is important also, as I feel most have a higher understanding of REST v.s. SOAP (myself included here :P).

jyokum’s picture

jyokum commented

I have a branch created which has a basic REST implementation. Not exactly the same as @mccrodp fork but carried over some of it. https://github.com/MarketoMA/marketo_ma/tree/2744087-rest-api

My goal with this branch is to do a 1:1 swap of the client and avoid any other changes. Still has some flaws but is basically working.

rolodmonkey’s picture

rolodmonkey commented

@jyokum: What do you need to turn this into a 7.x-2.x branch?

jyokum’s picture

jyokum commented

@RoloDMonkey I'm going to do few more updates to it today then i'll get the 7.x-2.x branch created.

mccrodp’s picture

mccrodp commented

Sounds good @jyokum. If you need a hand with anything, let me know. I didn't take that close a look yet, but two questions:

What did you do for mapping REST / SOAP field naming? I see you are still using MARKETO_MA_WEBFORM_FIELD_DEFAULTS, we got rid of that in the Marketo REST fork in favour of separating out the "Field Definitions" and mapping in the DB via the fields Marketo ID. See issue: https://github.com/mccrodp/marketo_rest/issues/6 and PR: https://github.com/mccrodp/marketo_rest/pull/11

Also, will your MarketoClient class need namespacing or is it ok having 2 classes the same name? I think I ran into issues with that initially, but didn't spend too long on it as I removed SOAP client pretty quickly. It wasn't clear to me how you were instantiating a specific client, but I guess there was some more work there as the interface was duplicated and not implemented by the rest client class yet, etc.

Good job anyways, roll on 2.x ;)

jyokum’s picture

jyokum commented

Since we're replacing SOAP with REST there's no need to keep the old SOAP client around. Haven't run into an issue still having the file in there but will be removing it entirely. The interface class isn't really useful and I'm considering removing it as well.

I haven't addressed Munchkin vs REST field names yet and I've been looking at how you handled it. I'm inclined to follow your approach. I like the table layout you have on config page as well.. that textarea was always a bit of a shortcut that needed to be addressed.

mccrodp’s picture

mccrodp commented

Since we're replacing SOAP with REST there's no need to keep the old SOAP client around.

Thanks for the clarification, I wasn't 100% on the goal after seeing it there.

I haven't addressed Munchkin vs REST field names yet and I've been looking at how you handled it. I'm inclined to follow your approach.

Sounds good to me, I'll see about contacting someone re: removing the Marketo REST project page once you have a 2.x version up. Was v.close to making a dev release on it, glad I didn't now as it's probably easier to get it removed with no releases. I will leave the Marketo REST module on GitHub and add info to the README to point back to Marketo MA once the 2.x is there also.

Good work, chat soon.

jyokum’s picture

jyokum commented

7.x-2.x branch has been cut. My preference is to get pull requests on GitHub but can deal with patches here as well.

https://github.com/MarketoMA/marketo_ma/tree/7.x-2.x

jyokum’s picture

jyokum commented
Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Active » Needs work
rolodmonkey’s picture

rolodmonkey commented

I am totally down with pull requests.

I see that you set the status to "Needs work". What is left to do?

jyokum’s picture

jyokum commented

For the most part it should be working. Here's what I know is outstanding.

  • The entire Behat test suite needs updating.
  • The module update hook in marketo_ma.install works pretty well but concerned I've overlooked something. I know it doesn't migrate previous field setup.
  • Marketo fields which are designated as read-only shouldn't be usable in user/webform field mappings
  • Need to improve try/catch error handling throughout the new REST client and associated functions.
  • Haven't validated Rules integration
  • README needs updating as well as documentation pages https://drupal.org/node/2057217
  • Need to implement Associate Lead somewhere in the flow. Where it's at right now it's losing the cookie/lead connection which was previously handled as part of syncLead
  • getLead is just returning the default fields, need to either return all fields or at a min the ones which have been enabled in config.
  • Ability to use Marketo partitions is likely not working

Beyond that just needs thorough testing.

jyokum’s picture

jyokum commented

I've addressed most of the remaining items in the latest push and will be logging new issues for the items I know need to be address.

Resolved:

  • Marketo fields which are designated as read-only shouldn't be usable in user/webform field mappings
  • Haven't validated Rules integration
  • Need to implement Associate Lead somewhere in the flow. Where it's at right now it's losing the cookie/lead connection which was previously handled as part of syncLead
  • getLead is just returning the default fields, need to either return all fields or at a min the ones which have been enabled in config.

Outstanding and new issues being created

jyokum’s picture

jyokum commented
Status: Needs work » Needs review
mccrodp’s picture

mccrodp commented

Status: Needs work » Needs review

I'm back on the project I was using Marketo REST module on now. I will be reworking that project to use Marketo MA now. I have requested removal of Marketo REST from Drupal.org #2816947: Removal of Marketo REST project.

Hopefully we can get this reviewed properly and I'll create any PRs for anything I find incl. hopefully addressing this in the process #2794419: Update README to account for changes in 7.x-2.x.

mccrodp’s picture

mccrodp commented

This looks good to me once we get this fixes / PRs in:

  1. https://github.com/MarketoMA/marketo_ma/pull/18
  2. https://github.com/MarketoMA/marketo_ma/pull/16
  3. https://github.com/MarketoMA/marketo_ma/pull/14
  4. https://github.com/MarketoMA/marketo_ma/pull/20
  5. https://github.com/MarketoMA/marketo_ma/pull/19

Thanks a lot for your hard work getting the REST functionality in it's right place here, @jyokum.