status of merges into core master

[Heather Piwowar]

Hi Kevin, 

I've been doing development+refactoring in the newproviders branch, streamlining the provider-specific code in anticipation of implementing many more providers (hopefully tonight).  

Some of these changes have impacted the core beyond the providers subdirectory.  I want to take on the merging burdon, so I'm holding off merging into master until you have merged in your major changes for this sprint.  

Let me know when your big changes are all in master, and then I'll port mine?

Thanks!

Heather

[Kevin Campbell]

Heather,

Is your latest code committed on the branch? If so, I'll see if it crosses over with anything else I've been working on.

Regards,
Kevin

Heather

--
You received this message because you are subscribed to the Google Groups "total-impact-dev" group.
To post to this group, send email to total-im...@googlegroups.com.
To unsubscribe from this group, send email to total-impact-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/total-impact-dev?hl=en.

[Kevin Campbell]

Looking over the code there, a few things pop out.

functional_tests and load_tests should really be merged where possible. It's a little tricky though as the code for the load_test is doing async loading of pages, so it's going to have differences. I think the comparison checks could certainly be shared though.

It seems we have lost query_type from members?

Is there a reason for moving the cache settings? Is this as some providers will need to have cache disabled?

And it looks like we've going with aliases calling get_aliases_for_id on each of the alias items (similar for metrics, etc). I think that's probably going to be useful for a number of providers.

Looks good to merge. I think the only real thing I'm going to have to change is the developers doc for providers. They'll probably need to grow a bit to encompass the additional structure, but that should be quick.

Regards,
Kevin

[Heather Piwowar]

functional_tests and load_tests should really be merged where possible.

yes agreed.  I'd started on my refactor of functional_tests before I noticed load_test (maybe was on a branch I didn't look at), and I haven't had a chance to go think about how to combine yet.   

It seems we have lost query_type from members?

Yes.  We've decreased the number of supported member_item types we support now that our product has more focus... in agreement w Jason last week we decided to take advantage of that and simplify the code as well since no provider uses multiple query_types.

 

Is there a reason for moving the cache settings? Is this as some providers will need to have cache disabled?

The reason was so that an individual call could have its cache disabled... if it is a config parameter, the asynchronous nature seems to me that we don't know exactly what is cached and what isn't.  That means that we need a whole uncached instance of ti backend to do uncached tests, which I'd rather avoid.  Passing in the cache settings to the provider methods themselves works around this.  

I'm not 100% sold on this approach and in a few places my code for doing it is a bit ugly and in need of refactoring, so please suggest/make improvements if you see them!

And it looks like we've going with aliases calling get_aliases_for_id on each of the alias items (similar for metrics, etc). I think that's probably going to be useful for a number of providers.

yes, I've tried to streamline this code to make it easier for other providers.  Also, I personally like having the /providers api option available for debugging and testing

Looks good to merge. I think the only real thing I'm going to have to change is the developers doc for providers. They'll probably need to grow a bit to encompass the additional structure, but that should be quick.

Cool, thanks Kevin!  

Heather

[Kevin Campbell]

On Sun, May 13, 2012 at 11:20 PM, Heather Piwowar <hpiw...@gmail.com> wrote:

functional_tests and load_tests should really be merged where possible.

yes agreed.  I'd started on my refactor of functional_tests before I noticed load_test (maybe was on a branch I didn't look at), and I haven't had a chance to go think about how to combine yet.

I'll merge some of that tomorrow.

Is there a reason for moving the cache settings? Is this as some providers will need to have cache disabled?

The reason was so that an individual call could have its cache disabled... if it is a config parameter, the asynchronous nature seems to me that we don't know exactly what is cached and what isn't.  That means that we need a whole uncached instance of ti backend to do uncached tests, which I'd rather avoid.  Passing in the cache settings to the provider methods themselves works around this.  

I'm not 100% sold on this approach and in a few places my code for doing it is a bit ugly and in need of refactoring, so please suggest/make improvements if you see them!

I think we can assume the cache won't change any functionality. We have some tests that the cache is correct, but we won't need to do the test suite both cached and uncached for other parts.

K