gouline / dbt-metabase

dbt + Metabase integration
https://pypi.org/project/dbt-metabase/
MIT License
442 stars 63 forks source link

IndexError: list index out of range #81

Closed nicolasberbigier closed 2 years ago

nicolasberbigier commented 2 years ago

Hello There seems to be a size limit somewhere dbt-metabase used to work but as the model grew it suddently stopped working I get

Traceback (most recent call last):
  File "/usr/local/bin/dbt-metabase", line 5, in <module>
    dbtmetabase.main()
  File "/usr/local/lib/python3.8/dist-packages/dbtmetabase/__init__.py", line 720, in main
    cli(max_content_width=600)  # pylint: disable=unexpected-keyword-arg
  File "/usr/lib/python3/dist-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python3/dist-packages/click/core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/local/lib/python3.8/dist-packages/dbtmetabase/__init__.py", line 113, in invoke
    return super().invoke(ctx)
  File "/usr/lib/python3/dist-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python3/dist-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/usr/local/lib/python3.8/dist-packages/dbtmetabase/__init__.py", line 262, in wrapper
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.8/dist-packages/dbtmetabase/__init__.py", line 568, in models
    dbt_models, aliases = dbt.read_models(
  File "/usr/local/lib/python3.8/dist-packages/dbtmetabase/models/interface.py", line 134, in read_models
    return self.parser.read_models(self, include_tags, docs_url)
  File "/usr/local/lib/python3.8/dist-packages/dbtmetabase/parsers/dbt_manifest.py", line 112, in read_models
    self._read_model(
  File "/usr/local/lib/python3.8/dist-packages/dbtmetabase/parsers/dbt_manifest.py", line 213, in _read_model
    depends_on_id = list(
IndexError: list index out of range
gouline commented 2 years ago

Impossible to tell without seeing the input model. Isolate which of your models is causing this and post it here (removing any sensitive information), then we can have a look. Judging by the line that it fails on, it would be something with a relationship to another model.

mihama commented 2 years ago

@nicolasberbigier did you by any chance introduce a self-referencing model recently? I just ran into the same error message, and by inspecting the manifest parser I realized that the current model name is excluded from the list of dependencies, resulting in an empty list for self-referencing relationship tests. Trying to access the first element of that empty list then produces the error we've both seen.

In our case, the self-referencing column is coming out of our engineering systems. It sounds strange, but the table is effectively a series of events, and it has an optional reference_event_id column that references the "preceeding event that caused this event". And in dbt we are testing to ensure that "every referenced_event_id actually exists in the table" -- by having a self-referencing relationship test:

models:
  - name: events

    columns:
    - name: event_id

    - name: reference_event_id
       tests:
        - relationships:
           to: ref('events')
           field: event_id

I would personally say that for testing, it is legitimate to establish "this id from this table exists again in another column of the same table". But for mapping foreign key relationships, that doesn't necessarily make sense. I guess the disconnect comes from the fact that we're trying to derive two different logical concepts from the same relationship test definition here.

So if you have a similar situation of self-referencing relationship tests, a hotfix/workaround might be to disable that relationship test for now to at least get dbt-metabase operational again for you.

Since this seems to be a pattern that can appear in the wild, I think it could also be useful to make the parsing logic a bit "safer": skip the relationship if the list of dependencies consists only of self-references instead of assuming that the list will always be of length = 1 even after the - {model["unique_id"]} operation.

z3z1ma commented 2 years ago

Agree,

We inherited the method of resolving fk ref using relationship test via manifest but I think we should put eyes on this. I will take a look at this if no one else beats me to it. I'm sure we'd run into it at my company at some point.

gouline commented 2 years ago

Related to #74