Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task visualiser: time values of D3 graph edges don't make sense? #2620

Closed
MichaelGrupp opened this issue Jan 15, 2019 · 5 comments
Closed

Task visualiser: time values of D3 graph edges don't make sense? #2620

MichaelGrupp opened this issue Jan 15, 2019 · 5 comments

Comments

@MichaelGrupp
Copy link
Contributor

MichaelGrupp commented Jan 15, 2019

When using the D3 graph visualization, the time values at the edges don't seem to correspond to the actual duration of that task. The code indicates that it's a duration time.

Here's an example script. It runs a TestTask which wraps 4 DummyTasks that each create a temporary file and sleep for 1 second.

So, running the whole pipeline should take a bit more than 4 seconds in total.

import time

import luigi


class DummyTask(luigi.Task):
    tmp_path = luigi.Parameter()

    def output(self):
        return luigi.LocalTarget(path=self.tmp_path, is_tmp=True)

    def run(self):
        open(self.tmp_path, 'w').write("")
        time.sleep(1)


class TestTask(luigi.WrapperTask):
    def requires(self):
        for path in ["/tmp/a", "/tmp/b", "/tmp/c", "/tmp/d"]:
            yield DummyTask(tmp_path=path)


if __name__ == "__main__":
    luigi.build([TestTask()])

When I run a luigid daemon and then start the script:

time python test.py

time reports a bit more than 4 seconds as expected.

However, the D3 graph in the web viewer shows this:
screenshot from 2019-01-15 18-32-11

To add to the confusion, not even the luigid daemon was running as long as any of the shown times in this example.

Could this be a time conversion bug or am I understanding something wrong?

@MichaelGrupp
Copy link
Contributor Author

I'm using luigi 2.8.2 from PYPI on Ubuntu 16.04

@dlstadther
Copy link
Collaborator

@MichaelGrupp Could you @-blame the author of that section of code? I'm not familiar with the visualization integrations.

@MichaelGrupp
Copy link
Contributor Author

@hadesbox can you help here?

The oldest occurence of this duration I could track down across file changes is here: 755064b#diff-c32982ed29d088f880fba9a28a99c1a7R294

MichaelGrupp added a commit to magazino/luigi that referenced this issue Jan 16, 2019
MichaelGrupp added a commit to magazino/luigi that referenced this issue Jan 16, 2019
Fixes the duration calculation. Both date objects `startTime` and
`finishTime` are in milliseconds, so the additional multiplication with
1000 was wrong.

Also, the duration is now calculated with:
   duration = last_updated - time_running
as it is done in datadog_metric.py:83

I decided to not display the time if the duration is longer than a day,
because then it gets messy with the Date objects (would require a
conversion to day of year, not day of month).
MichaelGrupp added a commit to magazino/luigi that referenced this issue Jan 16, 2019
Fixes the duration calculation. Both date objects `startTime` and
`finishTime` are in milliseconds, so the additional multiplication with
1000 was wrong.

Also, the duration is now calculated with:
   duration = last_updated - time_running
as it is done in datadog_metric.py:83

I decided to not display the time if the duration is longer than a day,
because then it gets messy with the Date objects (would require a
conversion to day of year, not day of month).
MichaelGrupp added a commit to magazino/luigi that referenced this issue Jan 16, 2019
Fixes the duration calculation. Both date objects `startTime` and
`finishTime` are in milliseconds, so the additional multiplication with
1000 was wrong.

Also, the duration is now calculated with:
   duration = last_updated - time_running
as it is done in datadog_metric.py:83

I decided to not display the time if the duration is longer than a day,
because then it gets messy with the Date objects (would require a
conversion to day of year, not day of month).
MichaelGrupp added a commit to magazino/luigi that referenced this issue Jan 16, 2019
Fixes the duration calculation. Both date objects `startTime` and
`finishTime` are in milliseconds, so the additional multiplication with
1000 was wrong.

Also, the duration is now calculated with:
   duration = last_updated - time_running
as it is done in datadog_metric.py:83

I decided to not display the time if the duration is longer than a day,
because then it gets messy with the Date objects (would require a
conversion to day of year, not day of month).
@MichaelGrupp
Copy link
Contributor Author

I think I fixed it. Will open a PR.

screenshot from 2019-01-16 18-40-15

MichaelGrupp added a commit to magazino/luigi that referenced this issue Jan 16, 2019
Fixes the duration calculation. Both date objects `startTime` and
`finishTime` are in milliseconds, so the additional multiplication with
1000 was wrong.

Also, the duration is now calculated with:
   duration = last_updated - time_running
as it is done in datadog_metric.py:83

I decided to not display the time if the duration is longer than a day,
because then it gets messy with the Date objects (would require a
conversion to day of year, not day of month).
@Tarrasch
Copy link
Contributor

Would be nice if the strings eventually get human formatted. But it's a good start to not have incorrect data. :)

MichaelGrupp added a commit to magazino/luigi that referenced this issue Mar 13, 2019
Fixes the duration calculation. Both date objects `startTime` and
`finishTime` are in milliseconds, so the additional multiplication with
1000 was wrong.

Also, the duration is now calculated with:
   duration = last_updated - time_running
as it is done in datadog_metric.py:83

I decided to not display the time if the duration is longer than a day,
because then it gets messy with the Date objects (would require a
conversion to day of year, not day of month).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants