koji

#1417 notification's optouts
Merged by mikem. Opened by tkopecek.
tkopecek/koji issue1204  into  master

Download 1417.patch

Related: https://pagure.io/koji/issue/1204

What API should handle this? Should we create new calls/CLI or modify existing create/list/deleteNotification ?
It looks to me better to add new calls, but it can be confusing for users what is the difference here.

pretty please pagure-ci rebuild

Should we create new calls/CLI or modify existing create/list/deleteNotification ?

I think separate calls and commands. Seems like it would be messy to cram the two things together.

I can see a possible exception for the list-notifications cli command. It might make sense for that to list blocks.

1 new commit added

  • API/CLI/tests for notification blocks

Added getBuildNotificationBlocks, getBuildNotificationBlock, createNotificationBlock, deleteNotificationBlock to API and block-notification, unblock-notification to CLI + extended list-notifications to show also blocks. I think, that potential edit-notification-block is not much useful here, but it can be added also.

Testing gives:

Traceback (most recent call last):
  File "/home/mike/Devel/koji/koji/hub/kojixmlrpc.py", line 236, in _wrap_handler
    response = handler(environ)
  File "/home/mike/Devel/koji/koji/hub/kojixmlrpc.py", line 279, in handle_rpc
    return self._dispatch(method, params)
  File "/home/mike/Devel/koji/koji/hub/kojixmlrpc.py", line 316, in _dispatch
    ret = koji.util.call_with_argcheck(func, params, opts)
  File "/home/mike/Devel/koji/koji/koji/util.py", line 263, in call_with_argcheck
    return func(*args, **kwargs)
  File "/home/mike/Devel/koji/koji/hub/kojihub.py", line 7610, in get_notification_recipients
    optouts = [r['user_id'] for r in query.execute()]
  File "/home/mike/Devel/koji/koji/hub/kojihub.py", line 8206, in execute
    data = _multiRow(query, self.values, (self.aliases or self.columns))
  File "/home/mike/Devel/koji/koji/hub/kojihub.py", line 4680, in _multiRow
    return [dict(zip(fields, row)) for row in _fetchMulti(query, values)]
  File "/home/mike/Devel/koji/koji/hub/kojihub.py", line 4652, in _fetchMulti
    c.execute(query, values)
  File "/home/mike/Devel/koji/koji/koji/db.py", line 136, in execute
    ret = self.cursor.execute(operation, parameters)
ProgrammingError: can't adapt type 'set'

-    user_ids = set([r['user_id'] for r in recipients])
+    user_ids = list(set([r['user_id'] for r in recipients]))

1 new commit added

  • use list for db instead of set

added commit

Anyway, interesting as it is not problem not only for me (f29) , but also for jenkins' envs.

1 new commit added

  • fix tests

rebased onto 45357e76c62a1dd5318f4a13b07b56602009e326

Anyway, interesting as it is not problem not only for me (f29) , but also for jenkins' envs.

I got that by calling get_notification_recipients directly (added a temporary export for it and called with fakehub script). I.e. not any of the unit tests.
Edited by mikem

flake8 found a few items

tests/test_hub/test_notifications.py:450:9: F841 local variable 'user_id' is assigned to but never used
tests/test_hub/test_notifications.py:790:5: F811 redefinition of unused 'test_deleteNotification_missing' from line 449
tests/test_hub/test_notifications.py:791:9: F841 local variable 'user_id' is assigned to but never used
tests/test_hub/test_notifications.py:800:5: F811 redefinition of unused 'test_deleteNotification_not_logged' from line 459
tests/test_hub/test_notifications.py:817:5: F811 redefinition of unused 'test_deleteNotification_no_perm' from line 476

I think the stray user_id assigns are probably copied from elsewhere, but the test redefinitions are a problem.

https://github.com/mikem23/koji-playground/commits/pagure/pr/1417

If the above changes look good, I can merge this.

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

:thumbsup:

Commit 916d03e9 fixes this pull-request

Pull-Request has been merged by mikem
Metadata
None

testing-ready

Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.