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

🚀 Raise ValueError if response is empty in CouldForCustomers source #900

Draft
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

adrian-wojcik
Copy link
Contributor

Summary

Adding new logic to raise ValueError with proper message if response from data source is empty

Importance

It is important for debugging process, as now it is more clear what is the issue if reposne will be empty

Checklist

This PR:

  • [ X] follows the guidelines laid out in CONTRIBUTING.md
  • links relevant issue(s)
  • [X ] adds/updates tests (if appropriate)
  • adds/updates docstrings (if appropriate)
  • [X ] adds an entry in CHANGELOG.md

Copy link
Contributor

@Rafalz13 Rafalz13 left a comment

Choose a reason for hiding this comment

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

Added comments.

viadot/sources/cloud_for_customers.py Outdated Show resolved Hide resolved
viadot/sources/cloud_for_customers.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@winiar93 winiar93 left a comment

Choose a reason for hiding this comment

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

please also apply changes for function _to_records_other

@@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

### Changed

- Changed `CloudForCustomers` methods: `_to_records_report` and `_to_records_other` to rasie ValueError if response from data source is empty
Copy link
Contributor

Choose a reason for hiding this comment

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

Description doesn't match with code modification

@@ -90,7 +90,6 @@ def _to_records_report(self, url: str) -> List[Dict[str, Any]]:
records.extend(new_records)

url = response_json["d"].get("__next")
Copy link
Contributor

Choose a reason for hiding this comment

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

@trymzet trymzet changed the title 🚀 Adding new logic to raise ValueError with proper message if response from data source is empty in C4C source 🚀 Raise ValueError if response is empty in CouldForCustomers source Sep 27, 2024
@k-wolski k-wolski marked this pull request as draft October 11, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants