Extensions Docs Server: Fix handling of nodocs

Extensions Docs Server: Fix handling of nodocs

The conversion script will now not generate templates for APIs marked as nodoc.
The server also handles them gracefully if they slip in and 404s.

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

[cduvall, 2012-08-08 22:57:14]

Fixed the nodoc problem.

[not at google - send to devlin, 2012-08-09 05:11:36]

The result lg but nodoc doesn't seem like an Exception to me.

http://codereview.chromium.org/10854054/diff/2001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

http://codereview.chromium.org/10854054/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_data_source.py:96: except Exception:
why did you change these from OSError to Exception? Exception will catch
programming errors, which we don't want to do. The only thing we should catch is
file errors like OSError.

(come to think of it, we should make sure that the various FileSystem classes
agree on what happens if a file isn't found... throw the same kind of exception,
return None, whatever)

In any case, see comment in handlebar_dict_generator.py. Shouldn't have a
NoDocError.

http://codereview.chromium.org/10854054/diff/2001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/converter.py (right):

http://codereview.chromium.org/10854054/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/converter.py:33: 'devtools', # Has an
intro, but marked as nodoc.
You moved this to articles right? So this shouldn't be needed?

http://codereview.chromium.org/10854054/diff/2001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/handlebar_dict_generator.py (right):

http://codereview.chromium.org/10854054/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/handlebar_dict_generator.py:41: raise
NoDocError()
I don't think an exception is the answer here. We should just have
self._namespace = None and handle that throughout here (e.g. return an empty
object from Generate, or one with just samples).

Let the lack of documentation be handled at the templates/public/... level.

http://codereview.chromium.org/10854054/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/handlebar_dict_generator.py:44: except
Exception as e:
why do we need to log this error rather than handling it at the top (which will
presumably log it, so we'll end up logging twice)?

http://codereview.chromium.org/10854054/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/handlebar_dict_generator.py:84: except
Exception as e:
ditto

http://codereview.chromium.org/10854054/diff/2001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/template_data_source.py (right):

http://codereview.chromium.org/10854054/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/template_data_source.py:120: return ''
See comment in handlebar_dict_generator.py. This file shouldn't need to change.

[cduvall, 2012-08-09 17:54:05]

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/api_data_source.py:96: except Exception:
On 2012/08/09 05:11:36, kalman wrote:
> why did you change these from OSError to Exception? Exception will catch
> programming errors, which we don't want to do. The only thing we should catch
is
> file errors like OSError.
> 
> (come to think of it, we should make sure that the various FileSystem classes
> agree on what happens if a file isn't found... throw the same kind of
exception,
> return None, whatever)
> 
> In any case, see comment in handlebar_dict_generator.py. Shouldn't have a
> NoDocError.

Yeah, I agree that the FileSystems should all behave the same if a file isn't
found. I'll do this in a separate patch and make a bug.

I had changed this to Exception because I don't believe the SubversionFileSystem
will throw an OSError when a file isn't found, so I was worried about that
exception not getting caught.

Bug here: http://crbug.com/141664

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
File chrome/common/extensions/docs/server2/converter.py (right):

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/converter.py:33: 'devtools', # Has an
intro, but marked as nodoc.
On 2012/08/09 05:11:36, kalman wrote:
> You moved this to articles right? So this shouldn't be needed?

This is still needed because devtools.json is in the api folder, so the script
will try to generate an API page. Since it should be generating an articles page
instead, it should be in |IGNORED_FILES|.

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
File chrome/common/extensions/docs/server2/handlebar_dict_generator.py (right):

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/handlebar_dict_generator.py:41: raise
NoDocError()
On 2012/08/09 05:11:36, kalman wrote:
> I don't think an exception is the answer here. We should just have
> self._namespace = None and handle that throughout here (e.g. return an empty
> object from Generate, or one with just samples).
> 
> Let the lack of documentation be handled at the templates/public/... level.

Done.

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/handlebar_dict_generator.py:44: except
Exception as e:
On 2012/08/09 05:11:36, kalman wrote:
> why do we need to log this error rather than handling it at the top (which
will
> presumably log it, so we'll end up logging twice)?

Done.

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/handlebar_dict_generator.py:84: except
Exception as e:
On 2012/08/09 05:11:36, kalman wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
File chrome/common/extensions/docs/server2/template_data_source.py (right):

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/template_data_source.py:120: return ''
On 2012/08/09 05:11:36, kalman wrote:
> See comment in handlebar_dict_generator.py. This file shouldn't need to
change.

Done.

[not at google - send to devlin, 2012-08-10 04:48:21]

lgtm

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
File chrome/common/extensions/docs/server2/converter.py (right):

https://chromiumcodereview.appspot.com/10854054/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/converter.py:33: 'devtools', # Has an
intro, but marked as nodoc.
On 2012/08/09 17:54:05, cduvall wrote:
> On 2012/08/09 05:11:36, kalman wrote:
> > You moved this to articles right? So this shouldn't be needed?
> 
> This is still needed because devtools.json is in the api folder, so the script
> will try to generate an API page. Since it should be generating an articles
page
> instead, it should be in |IGNORED_FILES|.

Sorry my mistake, got this confused with the integration tests.

https://chromiumcodereview.appspot.com/10854054/diff/1025/chrome/common/exten...
File chrome/common/extensions/docs/server2/handlebar_dict_generator.py (right):

https://chromiumcodereview.appspot.com/10854054/diff/1025/chrome/common/exten...
chrome/common/extensions/docs/server2/handlebar_dict_generator.py:69: return
None
Is it useful to still return the samples?

And yeah, would rather return {} than None either way.

[cduvall, 2012-08-10 17:34:17]

https://chromiumcodereview.appspot.com/10854054/diff/1025/chrome/common/exten...
File chrome/common/extensions/docs/server2/handlebar_dict_generator.py (right):

https://chromiumcodereview.appspot.com/10854054/diff/1025/chrome/common/exten...
chrome/common/extensions/docs/server2/handlebar_dict_generator.py:69: return
None
On 2012/08/10 04:48:22, kalman wrote:
> Is it useful to still return the samples?
> 
> And yeah, would rather return {} than None either way.

Done.