Refactor APIDataSource to work with DataSourceRegistry

Refactor APIDataSource to work with DataSourceRegistry

BUG=275039
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=262792

[ahernandez, 2014-04-08 17:45:06]

Could you please take a look at these changes? Also, sorry if the cleanup I did
shouldn't have been with this patch.

https://codereview.chromium.org/228723005/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/server2/api_data_source.py (left):

https://codereview.chromium.org/228723005/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/api_data_source.py:562: def get(self,
api_name, disable_refs=False):
The only class using |disable_refs| was ReferenceResolver, but since
ReferenceResolver no longer relies on APIDataSource I decided to just remove all
the disable ref stuff.

[not at google - send to devlin, 2014-04-09 16:27:54]

lgtm with a tiny cleanup, and after making the Cron implementation just "pass".

https://codereview.chromium.org/228723005/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/server2/api_data_source.py (left):

https://codereview.chromium.org/228723005/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/api_data_source.py:562: def get(self,
api_name, disable_refs=False):
On 2014/04/08 17:45:06, ahernandez.miralles wrote:
> The only class using |disable_refs| was ReferenceResolver, but since
> ReferenceResolver no longer relies on APIDataSource I decided to just remove
all
> the disable ref stuff.

Awesome.

https://codereview.chromium.org/228723005/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/228723005/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/api_data_source.py:87: self._namespace =
api_models.GetModel(api_name).Get()
see comment below about cron. this is one of those operations that we should be
pulling async; and adding a Cron method to APIModels would do that.

https://codereview.chromium.org/228723005/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/api_data_source.py:471: self._file_system
= server_instance.host_file_system_provider.GetTrunk()
this doesn't need to go on |self| because it's only accessed in the constructor.

https://codereview.chromium.org/228723005/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/server2/api_data_source.py:528:
futures.extend([Future(value=self.get(api_name))
setting a value= for each of these defeats the purpose of making them futures.
you might as well have this as:

for api_name in self._api_models.GetNames():
  self.get(api_name)  # maybe you need the try/except around here
return Future(value=())

However, the real win will come when we can lift the asynchronous operations out
into a true Collect of async futures; typically for data sources that means:

- make an asynchronous version of .get() (say GetAsync if you want)
- the implementation of get() would be "return GetAsync().Get()"
- the implementation of the Cron method Collects those async futures.

In this particular case though, it's very complex. There is a lot of different
data that goes into here, for example,
- API models
- features
- some templates
- samples

all that should be lifted out. In the APIModels and Features cases we should
expose a Cron() method for each of those. For the "some templates" case we can
pick the templates that we actually need (intro table).

for GetAsync itself it'd be simplest to make _GetSchemaModel async on the
datastore access.

... the moral of this story, I think, is to just make the implementation of this
method "pass" for now -- since that's what it already is -- then implement the
proper efficient Cron method in a follow-up.

[ahernandez, 2014-04-09 20:17:36]

The CQ bit was checked by ahernandez.miralles@gmail.com

[commit-bot: I haz the power, 2014-04-09 20:17:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ahernandez.miralles@gmail.com/22872300...

[commit-bot: I haz the power, 2014-04-09 20:19:32]

Change committed as 262792