Add job start and end time

[leewesleyv]

Resolves #11

• Add start time to job data for the Docker launcher
• Add start time to job data for the K8s launcher
• Add end time to job data for the Docker launcher
• Add end time to job data for the K8s launcher
• Add unit tests
• Add documentation

[wvengen]

Nice!
Great to see some cleanup too (though some comments).
I still need to get used to the idea of introducing typing here. Maybe discuss this first at a standup, and then perhaps introduce later?

[wvengen]

Looking good, thanks!
I see you've added the first unit test. Great!
The API test is not a regular test, as it is meant to run against a running instance. I'm not really sure if it fits well into the general pytest infrastructure like this. What do you think?

I'd split the API tests (which can be run completely separate from scrapyd-k8s, and could even be run against a different implementation, iirc), and the unit tests, into separate CI jobs.

[wvengen]

Thank you for the PR!
In general, fine 👍 Some notes that may help!

I understand the impulse to change some things as you go through the code :) When the PR would be ready, I would expect that you'd either explain the reason for your changes that are not strictly related to this PR (preferably having them in separate commits, so you can easily filter them out later if needed), or move them to a different PR.

Keep it up!

[wvengen]

Can we make it a bit terser? (a separate heading breaks the flow of reading through the endpoints; this is just a detail you're only interested in if you want to know more about this endpoint)

--- a/README.md
+++ b/README.md
@@ -228,10 +228,7 @@ Lists spiders from the spider image's `org.scrapy.spiders` label.
 ### `listjobs.json` ([➽](https://scrapyd.readthedocs.io/en/latest/api.html#listjobs-json))
 
 Lists current jobs by looking at Docker containers or Kubernetes jobs.
-
-#### Limitations
-
-* **End time**; The job's end time will be populated only for the Kubernetes (k8s) launcher, provided the job finishes successfully and is not canceled. For Docker, this value will always be null.
+Note that `end_time` is not yet supported for Docker.
 
 ### ~~`delversion.json`~~ ([➽](https://scrapyd.readthedocs.io/en/latest/api.html#delversion-json))
 

[wvengen]

Otherwise, good to go!

[leewesleyv]

@wvengen Sorry, I did not see your suggestion. Do you think the change I've made is OK? Or should I change it to the suggestion?

[wvengen]

Yes, please use the diff I posted. Thanks!

[wvengen]

Super, thanks!