Code review comment for lp:~xaav/loggerhead/export-tarball

[info] You are already exposing information "can_export" into the menu.pt. Why not just expose the revision number there instead. So "exported_tarball_revno" can be None indicating it has no tarball to download, or can be a string indicating what URL (or part of url, or whatever) to use.

[needsinfo] This defaults to always offering download links. Which means that if this rolls out on Launchpad, we will be letting people download via tarball. It hasn't been clear to me if this was specifically approved for Launchpad. As such *I* would tend to change:

25 def __init__(self, branch, friendly_name=None, config={},
26 graph_cache=None, branch_link=None, is_root=False,
27 - served_url=_DEFAULT, use_cdn=False):
28 + served_url=_DEFAULT, use_cdn=False, export_tarballs=True):

To "export_tarballs=False". Then if Launchpad wants to expose tarballs, it can chose to do so. We can easily update the 'bzr serve --http' and './serve_branches' code so that they default to exposing tarballs, without changing Launchpad's policy without their explicit review of it.

[needsinfo] This does all the exporting as raw '.tar' files. Without a way to configure it. One idea would be to change the 'export_tarballs' flag. Instead of being a boolean, it could be "tarball_format=None". If the format is None, then we don't export tarballs. Otherwise it can be 'tar' or 'tar.gz', or whatever.

I'm certainly open to discussion on these points, but I do think we should have answers for them before we land this.