better endpoint reprs

[glyph]

It is often desirable to indicate where one is going to be connecting (let's say, in a log message) before one actually makes such a connection. Given that most attributes of endpoints are private (and, for these purposes, should remain so: we don't want to have IStreamClientEndpoint grow a host attribute which is just a made-up value for logging), it is annoying that the repr() and str() of these endpoints just give a memory address, rather than information about where they will be listening on or connecting to.

(It may be worthwhile to split this into one ticket per endpoint type to have nice small branches.)

[glyph]

I think this ticket is actually pretty easy; it's just making the attributes of the various classes in twisted/internet/endpoints.py visible in the results of their __repr__s. I'm adding the keyword after double-checking with #twisted.

[chhabraamit]

I mistakenly uploaded the same file twice , Sorry for that.

[chhabraamit]

I mistakenly uploaded the same file twice , Sorry for that.

[chhabraamit]

Replying to chhabraamit:

I mistakenly uploaded the same file twice , Sorry for that.

And also commented twice .!!:P

[exarkun]

Thanks!

A few comments about the patch:

1. Don't add new test files like test_existingthing2.py. 2 isn't really meaningful. If there's a good reason to have new tests in a new file, then it should be possible to come up with a good name for the new file. If you're just adding more tests for existing classes or functions, then it's almost certainly better to just add those tests to the existing test module.
2. The Twisted coding standard calls for:
   1. lines to be wrapped before column 80
   2. whitespace around operators (eg foo % bar, not foo%bar)
   3. whitespace after , (eg (foo, bar) not (foo,bar))
   4. All modules, classes, and functions to have docstrings. This is particularly important for tests, where the docstring needs to convey to future maintainers the intent of the test.
   5. Test methods to be named like test_foo, not testFoo or test_Foo.
3. twisted.internet is part of Twisted Core. The Twisted Core topfiles directory is twisted/topfiles, so the news fragment belongs there, rather than in twisted/internet/topfiles.
4. The news fragment is a bit verbose. For a minor feature like this that touches a lot of classes in a single module, or which implement a single interface, it's probably better to summarize more. In particular, avoiding the repetition is very desirable. News fragments are half advertising material, half changelog entry. The point is to let people know very quickly what is different in a new release. We should avoid making them read essentially the same thing 8 times in a row.
5. As far as the actual repr additions goes:
   1. The implementation is somewhat redundant. twisted.python.util.FancyStrMixin should help with this.
   2. Some of the things included in these reprs should not be included - they don't convey anything meaningful to the reader. For example, the reactor and the ssl context factory.
   3. Some parts of the behavior are not well tested. For example, passing "test1" as the ssl context factory when testing the SSL endpoint repr obscures the result that will happen in real usage. Passing an actual context factory would be more realistic, and probably also reveal that context factories do not have very good repr.
   4. The changes to _WrappingFactory and _WrappingProtocol seem out of scope, at least. I'm also not sure they benefit anyone. When would someone want to see the repr of one of these?

Thanks again for this! Sorry about the delay in the review.

[ashfall]

(In [35006]) Branching to 'better-endpoint-reprs-5633'

[ashfall]

 Build Results

The news file may need improvement, I'm running short of ideas to phrase it well.

[exarkun]

Thanks.

1. All of the classes that were switched from subclassing object to FancyStrMixin need to be switched to subclass both. :) FancyStrMixin is a classic class, so subclasses are also classic unless they also subclass a new-style class, such as object. Classic vs new-style is a stupid, boring distinction, but all new classes in Twisted must be new-style, and any existing classes in Twisted must be kept in their current style, be that classic or new-style. I forget whether class Foo(object, FancyStrMixin): or class Foo(FancyStrMixin, object): is more correct. You might find out through experimentation.
2. There should probably be a test for the above. See twisted.internet.test.test_core.ObjectModelIntegrationMixin.assertFullyNewStyle and the following ObjectModelIntegrationTest.test_newstyleReactor for inspiration on how endpoints might be tested for this feature.
3. Feel free to call the builtin repr in the unit tests, instead of the __repr__ method of each endpoint.
4. I think the reprs would all be improved by the removal of the leading _ on almost every attribute they display. You can do this by putting more information into the showAttributes attribute. Its elements may be strings, as they are now, or tuples like (attributeName, displayName, formatCharacter). displayName will let you clean up the display.
5. Also, I suggest displaying the UNIX endpoint's mode in octal instead of decimal, since octal is the convention for dealing with that value.

[ashfall]

(In [35263]) Branching to 'better-endpoint-reprs-5633-2'

[ashfall]

(In [35705]) Branching to 'better-endpoint-reprs-5633-3'

[Julian]

#4970 was a duplicate of this.