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.

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

Jump to bottom

Add support for custom decimal string formatter #125

Merged
merged 8 commits into from
Mar 3, 2021
Merged

Add support for custom decimal string formatter #125

merged 8 commits into from
Mar 3, 2021

Conversation

dmosorast
Copy link
Contributor

Description of change

This PR introduces a new concept: A Singer custom string format type that specifies a decimal value.

The reasoning behind this is to maintain decimal precision throughout ELT pipelines by converting to and from JSON strings and parsing with decimal.Decimal (in Python).

Manual QA steps

  • None, will write automated unittests to exercise this function.

Risks

  • Low, this is an additive feature.

Rollback steps

  • revert this branch

dmosorast
dmosorast commented Mar 3, 2021
View reviewed changes
Copy link
Contributor Author

@dmosorast dmosorast left a comment

Choose a reason for hiding this comment

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

Turns out I can't request changes since I opened this PR, but these are "Requested Changes" heh

singer/transform.py Outdated Show resolved Hide resolved
singer/transform.py Outdated Show resolved Hide resolved
tests/test_transform.py Outdated Show resolved Hide resolved
tests/test_transform.py Outdated Show resolved Hide resolved
Jacob Baca added 2 commits March 3, 2021 19:30
dmosorast
dmosorast commented Mar 3, 2021
View reviewed changes
Copy link
Contributor Author

@dmosorast dmosorast left a comment

Choose a reason for hiding this comment

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

One more change.

tests/test_transform.py Outdated Show resolved Hide resolved
dmosorast
dmosorast commented Mar 3, 2021
View reviewed changes
Copy link
Contributor Author

@dmosorast dmosorast left a comment

Choose a reason for hiding this comment

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

Looks good. Approved without actually being able to hit the checkbox, but I think this is good to go ✅ :shipit:

@jacobrobertbaca jacobrobertbaca merged commit 0892140 into master Mar 3, 2021
@jacobrobertbaca jacobrobertbaca deleted the feature/decimal-formatted-string-types branch March 3, 2021 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobrobertbaca jacobrobertbaca jacobrobertbaca approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dmosorast @jacobrobertbaca