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

With org: or owner: in SEARCH_QUERY the action fails #290

Closed
albertosantini opened this issue May 23, 2024 · 6 comments · Fixed by #291
Closed

With org: or owner: in SEARCH_QUERY the action fails #290

albertosantini opened this issue May 23, 2024 · 6 comments · Fixed by #291
Labels
bug Something isn't working

Comments

@albertosantini
Copy link

Describe the bug

If in SEARCH_QUERY I use

repo:my_org/my_project ...

the action works correctly.

If I use

org:my_org ... or owner:my_owner ...

it fails as the repositories are not listed.
See screenshot of action log.

To Reproduce

Using org: or owner: and not repo: keys in the SEARCH_QUERY.

Expected behavior

The action is executed

Screenshots

image

Additional context

No response

@albertosantini albertosantini added the bug Something isn't working label May 23, 2024
@albertosantini
Copy link
Author

The token seems ok to me.

image

@jmeridth
Copy link
Member

@albertosantini Thank you for filing this issue. We will take a look at this as soon as possible. Please let us know if you'd like to try out a PR for this. We don't want to block an opportunity for others to contribute fixes. 🙇

@albertosantini
Copy link
Author

I am not so proficient in Python.

The name of this test is misleading because there is not the repo in the query and, indeed, the repo is empty.

    def test_get_owner_and_repositories_with_repo_in_query(self):
        """Test get just owner."""
        result = get_owners_and_repositories("org:owner1")
        self.assertEqual(result[0].get("owner"), "owner1")
        self.assertIsNone(result[0].get("repository"))

Anyway the offending code is the following one:

    for item in owners_and_repositories:
        repos_and_owners_string += f"{item['owner']}/{item['repository']} "

And the test above needs to be improved to catch the case of the offending lines above.

Furthermore owners_and_repositories and the corresponding repos_and_owners_string are used only to display an error message when a github3 exception is raised, for instance:

f"The repository could not be found; Check the repository owner and names: '{repos_and_owners_string}"

I would change the logic in

def get_owners_and_repositories(
    search_query: str,
) -> List[dict]:

and not to return a List, but directly a string of owners and repositories.

@albertosantini
Copy link
Author

Maybe the image of the log is cropped, but the error is KeyError: 'repository'.

So another approach is testing if there is any owner AND repository key in the dictionary, before concatenating the string from the List.

    for item in owners_and_repositories:
        repos_and_owners_string += f"{item['owner']}/{item['repository']} "

@albertosantini
Copy link
Author

What about using a default value if the key doesn't exist (quick fix)?

repos_and_owners_string += f"{item['owner']}/{item.get('repository', '')} "

jmeridth added a commit that referenced this issue May 23, 2024
Fixes #290

- [x] handle trying to access repository from dictionary when not present (use default of empty string)
- [x] add test to ensure we cover this scenario
- [x] handle issue where `time_to_close` variable was being access before possibly being declared

Signed-off-by: jmeridth <[email protected]>
jmeridth added a commit that referenced this issue May 23, 2024
Fixes #290

- [x] handle trying to access repository from dictionary when not present (use default of empty string)
- [x] add test to ensure we cover this scenario
- [x] handle issue where `time_to_close` variable was being access before possibly being declared

Signed-off-by: jmeridth <[email protected]>
@albertosantini
Copy link
Author

Thanks @jmeridth.

Tested. Ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants