split the gem? #22
Comments
|
i would tend to think that's a bad idea... there's so much common code that is so important just in terms of sending the right HTTP requests, authorizations, etc... the real answer is to just get incredibly solid VCR specs in the repo. i added specs for almost everything i touched, but i'm sure there's a few holes here and there. |
|
It should be possible to have a common gem that wraps up the HTTP request and authorization stuff. I think it would be a lot smoother if there were a buyer and seller side gem, then you wouldn't have to bug us to get changes made that we are unable to really test (I get that VCR testing gets around this, but we still are unable to verify that it works, or work on fixing anything that might come up in the seller-side code) |
|
well, FWIW i am leaving my engagement at the place i updated the gem for on friday, so there won't be any more changes coming from me. i'm also pretty happy with where the gem is right now. in general i feel you on why it would be nice, and if the scope of the project were larger i would endorse that approach. but this is just a simple wrapper for some REST API calls - do you really want to be managing 3 separate libraries (core, seller side, buyer side) for that? it's a lot of overhead for very minor return IMHO. |
|
also i think a lot of the seller side stuff did not come from me - it's code added by other people over the years. i am very surprised that our account has seller side access, given that we are not a seller. |
|
I think that's a fair point, unless @mcmoyer really wants to do the work of splitting it up 😜 |
I'm not surprised... given that they have now consolidated to one, and synchronize production data to it, it'll probably be hard to get access to that. But you could ask about the older (unsupported, afaik, but it appears to have been updated to v1.17) sand.api.appnexus.com. By the way, is there any specs that still need recording? I'll probably be taking a look at the version we're using to make sure we're still compatible with v1.18 so I might be able to record some things. |
|
the sand.api.appnexus.com is what we used to use to test against, but it does seem that some of the tests are no longer working against that. Something I didn’t mention in the first post was that our testing endpoint is api-test.adnxs.com, whereas you guys are using api-test.appnexus.com — one more thing to work around |
|
re: breaking changes coming - see: #23 |
|
There was also a per https://wiki.appnexus.com/display/api/Client+Testing+Environment , |
|
just to keep you guys in the loop. I’m working on the updates for the v1.18 changes. I started with the creative service specs as thats our main service we use. The way the specs are running right now, it fails. The current specs reference the Advertiser service which we don’t have access to. My feeling with VCR specs is that they should always be runnable…and that they should periodically be deleted and refreshed. Maybe this could be solved by having ImpBus specs and Console API specs? |
|
Also realizing that the endpoints aren’t a one for one analogue Viewing a creative on ImpBus Viewing a creative on Console API |
I’ve been communicating with Appnexus about a test system with permissions for both buyer and seller sides. It does not look like this will be available.
That said, there’s now code in the test suite that is specific to the seller side that I’m unable to test fully on my end. I’m wondering if we should look into splitting this gem into two..one for the buyer side, one for the seller side?
Thoughts?
@apurvis @joshuawscott
The text was updated successfully, but these errors were encountered: