Gitlab worker fails because index out of range

[dhufe]

In my case I tried to fetch issues which are assigned to my user and I used

[gitlab_instance]
service = gitlab
gitlab.only_if_assigned = USER
gitlab.owned = True
gitlab.login = USER
gitlab.token = TOKEN
gitlab.host = HOST
gitlab.import_labels_as_tags = True
gitlab.label_template = GL {{label}}
gitlab.description_template = Issue {{gitlabnumber}}: {{gitlabtitle}}

At the moment there aren't any assigned issues and the response contains an empty list. I added some code and updated the tests the for only_if_assigned behavior in GitLab service. They handle some additional edge cases for user lookup, including missing, multiple matches, and also_unassigned.

With theses changes I got no error anymore and if I created some issues in the production system, it runs just fine.

If you have any further questions, feel free to contact me.

Bests
Daniel

[dhufe]

I fogort the traceback of the error:

Traceback (most recent call last):
  File "/home/dhufe/dev/bugwarrior/.venv/lib64/python3.12/site-packages/bugwarrior/collect.py", line 45, in _aggregate_issues
    service = get_service(conf[target].service)(conf[target], conf[main_section])
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dhufe/dev/bugwarrior/.venv/lib64/python3.12/site-packages/bugwarrior/services/gitlab.py", line 499, in __init__
    self.gitlab_client = GitlabClient(
                         ^^^^^^^^^^^^^
  File "/home/dhufe/dev/bugwarrior/.venv/lib64/python3.12/site-packages/bugwarrior/services/gitlab.py", line 147, in __init__
    self._fetch(f'users?username={only_if_assigned}')[-1]['id']
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^
IndexError: list index out of range

[ryneeverett]

At the moment there aren't any assigned issues and the response contains an empty list.

My understanding is that self._fetch(f'users?username={only_if_assigned}') is just looking up the username with the user api. Shouldn't the user be returned irrespective of whether any issues are assigned?

I'm sure there's some room for improvement here but I want to make sure we understand the issue first.

[dhufe]

Thanks for your reply. For sure, it only tries to query the user.

The problem is generated by [-1]['id'] and the case, that the user does not exist (or is incorrect). As far as I understand that would take the last element of a list, no matter if it's an empty one.

[ryneeverett]

IMO, if the only_if_assigned username does not exist, it ought to throw an error as it currently does rather than log a warning and continue. An improvement would be to catch the IndexErrror in that case and raise an exception with a more helpful error message.

I don't think we need to handle len(users) > 1 because according to the docs that shouldn't be possible:

Get a single user with a specific username.

And I think it is implied elsewhere in that doc that the username query should only return an "exact match".

[dhufe]

I don't think we need to handle len(users) > 1 because according to the docs that shouldn't be possible:

That’s a good point, but then -1 is also wrong. It implies directly, that the response might contain multiples users. And that’s ok since the Gitlab docs say:

Name or username, you do not have to get an exact match because this is a fuzzy search.

and therefore a response may contain multiple users.

And I think it is implied elsewhere in that doc that the username query should only return an "exact match".

From the bugwarrior docs:

only_if_assigned: If set to a username, only import issues assigned to the specified user.

I don’t think so, but may be I rushed over the text. My first thought was: why should this an user and why Bugwarrior do not use my gitlab.login, which was already defined (by the way if you use a Token the user does not matter anymore, the same for GitHub). I understand the use case to query stuff for a different user. May be it would be handy to allow strings and boolean.

[ryneeverett]

Name or username, you do not have to get an exact match because this is a fuzzy search.

This refers to the search parameter, not the username parameter.

My first thought was: why should this an user and why Bugwarrior do not use my gitlab.login, which was already defined (by the way if you use a Token the user does not matter anymore, the same for GitHub). I understand the use case to query stuff for a different user. May be it would be handy to allow strings and boolean.

The answer to a lot of "why does bugwarrior do x?" questions is historical. My recollection is that this feature existed before services started aggressively rate banning or disallowing unauthenticated api queries and authentication options were generally optional. And now basic user/password authentication is largely being phased out for api use so the login parameters are probably going to disappear eventually.

Good point about login not being needed with token authentication. Unfortunately, we're using the login to automatically namespace the 'include_repos' and 'exclude_repos' options and there doesn't seem like a great way to avoid that. It does look like it could be easily deleted from the github service though.

[dhufe]

The answer to a lot of "why does bugwarrior do x?" questions is historical. My recollection is that this feature existed before services started aggressively rate banning or disallowing unauthenticated api queries and authentication options were generally optional. And now basic user/password authentication is largely being phased out for api use so the login parameters are probably going to disappear eventually.

That wasn't my intention and this is normal and totally ok for grown projects.

Good point about login not being needed with token authentication. Unfortunately, we're using the login to automatically namespace the 'include_repos' and 'exclude_repos' options and there doesn't seem like a great way to avoid that. It does look like it could be easily deleted from the github service though.

I suggest keeping the current logic unchanged in this PR and only improving error handling. As a next step, I would:

add proper exception handling with meaningful log output
explicitly check the response length (0 → error, 1 → success)
document that GitLab returns at most one user for this query (verified against multiple instances)

Bests
Daniel

[ryneeverett]

add proper exception handling with meaningful log output

👍

explicitly check the response length (0 → error, 1 → success)

What advantage does this have over catching the IndexError?

document that GitLab returns at most one user for this query (verified against multiple instances)

I don't understand what this means.

[dhufe]

add proper exception handling with meaningful log output

👍

explicitly check the response length (0 → error, 1 → success)

What advantage does this have over catching the IndexError?

Using IndexError here would technically handle the empty case, but it hides the actual assumption we are making — namely, that the API must return exactly one user.
An explicit length check makes this expectation visible in the code and avoids using an exception as part of normal control flow. I believe this improves readability and long-term maintainability.

document that GitLab returns at most one user for this query (verified against multiple instances)

I don't understand what this means.

What I meant is simply documenting in the code that this GitLab endpoint returns a list that either:

contains exactly one element, or
is empty.

This would clarify the expected behavior and hopefully avoid further discussions like this in the future.

Best regards,
Daniel

[dhufe]

Ok, I updated the code:

Handle unresolved usernames gracefully by logging a warning instead of raising an error.
When the GitLab API does not return exactly one user, the assignee filter is intentionally ignored and the import continues unchanged.

Let me know if this addresses your feedback.

[dhufe]

Applied your suggested change and updated the tests accordingly.

[ryneeverett]

I'm guessing CI failed because of the extra newline left at the end of test_gilab.py. Could you remove it or run ruff format?