Skip to content

fix: some issues with data output to postgresql #63

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.

Sign up for GitHub

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

Merged
lalepee merged 1 commit into from
Jan 9, 2026
Merged

fix: some issues with data output to postgresql #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cosmotech/coal/postgresql/runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ def send_runner_metadata_to_postgresql(
(
runner.get("id"),
runner.get("name"),
runner.get("lastRunId"),
runner.get("lastRunInfo").get("lastRunId"),
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code uses chained .get() calls on nested dictionaries without checking if lastRunInfo exists or is None. If runner.get("lastRunInfo") returns None, calling .get("lastRunId") on it will raise an AttributeError. Consider using safe navigation or adding a check to handle cases where the nested structure might be incomplete.

Copilot uses AI. Check for mistakes.
runner.get("runTemplateId"),
),
)
conn.commit()
LOGGER.info(T("coal.services.postgresql.metadata_updated"))
return runner.get("lastRunId")
return runner.get("lastRunInfo").get("lastRunId")
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code uses chained .get() calls on nested dictionaries without checking if lastRunInfo exists or is None. If runner.get("lastRunInfo") returns None, calling .get("lastRunId") on it will raise an AttributeError. Consider using safe navigation or adding a check to handle cases where the nested structure might be incomplete.

Copilot uses AI. Check for mistakes.


def remove_runner_metadata_from_postgresql(
Expand Down
8 changes: 6 additions & 2 deletions cosmotech/coal/store/output/channel_spliter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

class ChannelSpliter(ChannelInterface):
requirement_string: str = "(Requires any working interface)"
targets = []
targets = list()
available_interfaces: dict[str, ChannelInterface] = {
"s3": AwsChannel,
"az_storage": AzureStorageChannel,
Expand All @@ -21,7 +21,7 @@ class ChannelSpliter(ChannelInterface):

def __init__(self, dct: Dotdict = None):
super().__init__(dct)
self.targets = []
self.targets = list()
if "outputs" not in self.configuration:
raise AttributeError(T("coal.store.output.split.no_targets"))
for output in self.configuration.outputs:
Expand All @@ -45,6 +45,8 @@ def send(self, filter: Optional[list[str]] = None) -> bool:
any_ok = i.send(filter=filter) or any_ok
except Exception:
LOGGER.error(T("coal.store.output.split.send.error").format(interface_name=i.__class__.__name__))
if len(self.targets) < 2:
raise
return any_ok

def delete(self, filter: Optional[list[str]] = None) -> bool:
Expand All @@ -54,4 +56,6 @@ def delete(self, filter: Optional[list[str]] = None) -> bool:
any_ok = i.delete() or any_ok
except Exception:
LOGGER.error(T("coal.store.output.split.delete.error").format(interface_name=i.__class__.__name__))
if len(self.targets) < 2:
raise
return any_ok
1 change: 1 addition & 0 deletions cosmotech/coal/store/output/postgres_channel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def send(self, filter: Optional[list[str]] = None) -> bool:
configuration=self.configuration,
selected_tables=filter,
fk_id=run_id,
replace=False,
)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The send method does not return a boolean value as expected by its type signature. The method should return True on success or False on failure to be consistent with the ChannelInterface contract and the calling code in ChannelSpliter that expects a boolean return value.

Suggested change
)
)
return True

Copilot uses AI. Check for mistakes.

def delete(self):
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/coal/test_postgresql/test_postgresql_runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def test_send_runner_metadata_to_postgresql(self, mock_connect, mock_postgres_ut
mock_runner = {
"id": "test-runner-id",
"name": "Test Runner",
"lastRunId": "test-run-id",
"lastRunInfo": {"lastRunId": "test-run-id"},
"runTemplateId": "test-template-id",
}

Expand Down Expand Up @@ -87,7 +87,7 @@ def test_send_runner_metadata_to_postgresql(self, mock_connect, mock_postgres_ut
assert upsert_call[0][1] == (
mock_runner["id"],
mock_runner["name"],
mock_runner["lastRunId"],
mock_runner["lastRunInfo"]["lastRunId"],
mock_runner["runTemplateId"],
)

Expand Down
15 changes: 8 additions & 7 deletions tests/unit/coal/test_store/test_output/test_channel_spliter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,11 @@ def test_send_with_exception(self):
# Act
with patch.dict(ChannelSpliter.available_interfaces, {"s3": mock_channel_class}):
spliter = ChannelSpliter(mock_config)
result = spliter.send()

# Assert
assert result is False
with pytest.raises(Exception):
result = spliter.send()
# Assert
assert result is False

def test_delete_success(self):
"""Test delete method when all targets succeed."""
Comment on lines +249 to 254
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion assert result is False will never be executed because it's placed after the line that raises an exception. When spliter.send() raises an exception, control immediately jumps to the exception handler, skipping this assertion. If the intent is to verify behavior when an exception is raised, this assertion should be removed.

Suggested change
result = spliter.send()
# Assert
assert result is False
def test_delete_success(self):
"""Test delete method when all targets succeed."""
spliter.send()
def test_delete_success(self):
"""Test delete method when all targets succeed."""
"""Test delete method when all targets succeed."""

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -331,10 +332,10 @@ def test_delete_with_exception(self):
# Act
with patch.dict(ChannelSpliter.available_interfaces, {"s3": mock_channel_class}):
spliter = ChannelSpliter(mock_config)
result = spliter.delete()

# Assert
assert result is False
with pytest.raises(Exception):
result = spliter.delete()
# Assert
assert result is False

Comment on lines +336 to 339
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion assert result is False will never be executed because it's placed after the line that raises an exception. When spliter.delete() raises an exception, control immediately jumps to the exception handler, skipping this assertion. If the intent is to verify behavior when an exception is raised, this assertion should be removed.

Suggested change
result = spliter.delete()
# Assert
assert result is False
spliter.delete()

Copilot uses AI. Check for mistakes.
def test_available_interfaces(self):
"""Test that available_interfaces are properly defined."""
Expand Down
Loading