apply coalesce to values being bulk-added to handle nulls #3354
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
augur/application/db/lib.py
Outdated
|
|
||
|
|
||
|
|
||
| def bulk_insert_dicts(logger, data: Union[List[dict], dict], table, natural_keys: List[str], return_columns: Optional[List[str]] = None, string_fields: Optional[List[str]] = None, on_conflict_update:bool = True) -> Optional[List[dict]]: |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0621: Redefining name 'logger' from outer scope (line 19) (redefined-outer-name)
sgoggins
added
discussion
MoralCode
force-pushed
the
null_cntrb_email
branch
2 times, most recently
from
436b6d3 to
f4f0501
Compare
|
|
||
|
|
||
|
|
||
| def bulk_insert_dicts(logger, data_input: Union[List[dict], dict], table, natural_keys: List[str], return_columns: Optional[List[str]] = None, string_fields: Optional[List[str]] = None, on_conflict_update:bool = True) -> Optional[List[dict]]: |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0621: Redefining name 'logger' from outer scope (line 19) (redefined-outer-name)
|
I did a quick smoke test and it looks like this PR fixes our problem with some commits, like UCL/rsd-engineeringcourse@fffaf9a, not being attributed to any user, as it's currently happening. |
|
Sweet! Since I made modifications since we wrote this and it impacts a critical part of the code, I'd like to get @ABrain7710's input on this before merging |
…pped Signed-off-by: Adrian Edwards <adredwar@redhat.com> Co-authored-by: Mahmoud Abdelrazek <44040283+razekmh@users.noreply.github.com> Co-authored-by: Andrew Brain <andrewbrain2019@gmail.com>
…bject as well. Co-Created by: gpt-5 via cursor Signed-off-by: Adrian Edwards <adredwar@redhat.com>
MoralCode
force-pushed
the
null_cntrb_email
branch
from
f4f0501 to
f5acdf5
Compare
5 participants
Description
A collaborative change built with the other augur maintainers (@sgoggins and @ABrain7710) to add the
coalescepostgres function to our bulk insert methods to ensure that values in the database cannot be overwritten with NULL values. This was inspired by @razekmh's solution in #3342This was implemented without a parameter to toggle this functionality off because we couldnt think of a scenario where one might want to intentionally overwrite data with null in augur
This PR fixes #3317
This supersedes #3342
Notes for Reviewers
should be all set to merge once the OP of the linked issue confirms this still fixes the issue.
Signed commits