Prefect: Standardize class __repr__'s

[jlowin]

As observed in https://github.com/PrefectHQ/prefect/pull/616#discussion_r253284001, class reprs are inconsistent:

Core objects have <> around them:

<Flow: name=Flow>
<Task: Task>
<Edge: Task to Task>

States are "called" types:

Success()
Failed()

Runners use Python's default:

<prefect.engine.flow_runner.FlowRunner at 0x10e8d6e80>

[cicdw]

Is the proposed standardization something of the form:

<className: short attribute description>

so, for example, State's would become:

<Success: message="All reference tasks succeeded">

?

[jlowin]

I personally like that approach, something like [forgive pseudocode]:

f"<{type(self).__name__}: {self.details}>"

basically

But for example I don't think using actual identifiers (like Task/Flow ID) is helpful -- more like a generic way to get a sense for "what" the object is. For states, message seems good if available.

[cicdw]

FYI just found out, if any of these objects are used as _defaults_ in a documented call signature, the </> will be interpreted as an HTML tag by vuepress. One such object is NoResult.

[cicdw]

Update: This is totally OK, you just need to additionally override __str__ with something HTML friendly. 💯

[nandahkrishna]

Is it alright if I work on this issue?

[cicdw]

Yea absolutely @nandahkrishna that'd be great! Let me know if you have any questions about which objects need updating, etc.

[nandahkrishna]

It would be great if you could give me an idea on which objects need updating. I just had a look at some of the class definitions and spotted a few (like Runner, which you have mentioned earlier).

[jlowin]

[cicdw]

Here's a list to get you started:

• all State objects in src/prefect/engine/state.py (you should only have to update the base State class __repr__)
• all Runner objects (TaskRunner, FlowRunner, CloudTaskRunner, CloudFlowRunner)
• all executors (DaskExecutor LocalExecutor, SyncExecutor)
• JSONResultHandler, LocalResultHandler, S3ResultHandler and GCSResultHandler would be nice to haves
• the base Storage class
• the base execution Environment class

I'll update here if I find any others or if @jlowin thinks of any that I'm missing.

EDIT: Updated with a few more. And to be clear, don't feel that you have to exhaustively go through each one of these to make a PR. Even incremental progress here would be great!

[nandahkrishna]

Thanks! I'll get started on this.

[jlowin]

I have a suspicion that for each type of class, only the base class really needs updating -- State, Runner, BaseExecutor.

[nandahkrishna]

I have a suspicion that for each type of class, only the base class really needs updating -- State, Runner, BaseExecutor.

I'll check first by updating the base classes alone, and then if required update the rest.

Thanks @cicdw for the updated list. And yes, I'll make a PR even if it has just a few changes - helps me when I should change stuff after you review it!

[nandahkrishna]

I've created a PR #1050 for this, and so far I've updated the base classes Runner and State, which are now represented as <Runner: name=Runner> and <State: name=State>. Do tell me if the format/details in the __repr__ have to be changed.

Edit: Tests for the __repr__s are failing, and I'll have to update them to pass the checks. If you're okay with this formatting, I'll go ahead and change them.

Edit 2: Updating State updates for it's derived classes as well, so I think it's safe to assume the same for the other classes as well.

[cicdw]

Closed via #1050