Transitive import in trac/web/main.py

[Tim Graham <timograham@…>]

The linked block references StringIO but it's not imported (unless it's coming from one of the import * imports (is it a goal to remove those?).

​https://github.com/edgewall/trac/blob/8eaafa64ad6555e80ad7b0f0cdec1f340ebd6146/trac/web/main.py#L257-L262

[Ryan J Ollos]

Replying to Tim Graham <timograham@…>:

The linked block references StringIO but it's not imported (unless it's coming from one of the import * imports (is it a goal to remove those?).

The code can be activated by appending ?hdfdump=1 to the URL, which enables a developer to view the values of the template parameters. The acronyms hdf is historical as far as I know, a holdover from when Trac used ClearSilver as the templating engine.

I think both your observations are correct. We shouldn't use from trac.web.api import *, especially since trac.web.api doesn't define __all__, and StringIO should be explicitly imported in trac.web.main. I'll fix the latter now.

[Ryan J Ollos]

[14184,14186] adds import of StringIO to trac.web.main. Merged to trunk in [14185] (and the log message should say 1.2dev rather than 1.1.7dev, I figured I would make that error before too long …).

[Ryan J Ollos]

Defining __all__ for trac.web.api results in this bit of ugliness.

trac/web/api.py

@@ -39 +39 @@
 from trac.web.href import Href
 from trac.web.wsgi import _FileWrapper
 
+__all__ = ['IAuthenticator', 'IRequestHandler', 'is_valid_default_handler',
+           'IRequestFilter', 'ITemplateStreamFilter', 'parse_arg_list',
+           'arg_list_to_args', 'RequestDone', 'Cookie', 'Request']
+
 
 class IAuthenticator(Interface):
     """Extension point interface for components that can provide the name
@@ -228 +232 @@
     setattr(sys.modules[__name__], exc_name,
             HTTPException.subclass(exc_name, code))
     _HTTPException_subclass_names.append(exc_name)
+__all__ += _HTTPException_subclass_names
 del code, exc_name

We could break convention and put __all__ at the end of the file.

[Ryan J Ollos]

Replying to rjollos:

We could break convention and put __all__ at the end of the file.

I'm thinking this is probably more clear.

trac/web/api.py

@@ -840 +840 @@
         for cookie in cookies.splitlines():
             self._outheaders.append(('Set-Cookie', cookie.strip()))
 
+
+__all__ = ['IAuthenticator', 'IRequestHandler', 'is_valid_default_handler',
+           'IRequestFilter', 'ITemplateStreamFilter', 'parse_arg_list',
+           'arg_list_to_args', 'RequestDone', 'Cookie', 'Request'] + \
+          _HTTPException_subclass_names
+
+
 __no_apidoc__ = _HTTPException_subclass_names

[Tim Graham <timograham@…>]

Personally, I wouldn't worry too much about defining __all__ and just eliminate import *.

[Ryan J Ollos]

Fixed in [14214]. Thanks Tim!

[Jun Omae]

I got AttributeError: class StringIO has no attribute 'StringIO' with hdfdump=1 parameter, #12151. I dislike uses of star imports :<