-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat(mappers): Stream map expressions now have access to the Faker
class, rather than just a faker instance
#2598
Conversation
…pper expressions (provided a faker config is found).
…he faker library to re-seed
CodSpeed Performance ReportMerging #2598 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2598 +/- ##
==========================================
- Coverage 89.66% 89.62% -0.05%
==========================================
Files 58 58
Lines 4817 4817
Branches 944 944
==========================================
- Hits 4319 4317 -2
- Misses 347 349 +2
Partials 151 151 ☔ View full report in Codecov by Sentry. |
Maybe I'm missing something, but could this not have been achieved already using stream_maps:
customers:
# will always generate the same value for the same seed
first_name: fake.seed_instance(_['first_name']) or fake.first_name()
faker_config:
# IMPORTANT: `fake` and `Faker` names are only available if faker_config is defined.
locale: en_US https://faker.readthedocs.io/en/master/index.html#seeding-the-generator |
Oh, right
If that's the case, we should deprecate Thoughts @Mac-lp3? |
Happy to do some testing tomorrow and see if it provides the same
functionality. I'll respond here with the results and some pros/cons if any.
…On Sun, Sep 15, 2024, 3:32 PM Edgar Ramírez Mondragón < ***@***.***> wrote:
Oh, right
Each generator can also be switched to use its own instance of
random.Random, separated from the shared one, by using the seed_instance()
method, which acts the same way.
If that's the case, we should deprecate Faker and recommend
seed_instance().
Thoughts @Mac-lp3 <https://github.com/Mac-lp3>?
—
Reply to this email directly, view it on GitHub
<#2598 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABG3QS6ATZS5GFS5QJXDAB3ZWT5WVAVCNFSM6AAAAABMI4HUIWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJRGMZTMOBUGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ReubenFrankel you are 100% correct, you can accomplish the exact same thing with @edgarrmondragon, so it looks like the mapper.py changes can be reverted... i think it would at least be handy to keep the updated docs that explains how to get consistent hashing and why you may want to. Of course, it just needs to be updated again to use Should I just submit a new PR for that? If so, should it just be for the documentation changes, or should it also include reverting |
Thanks for the offer @Mac-lp3! Can you take a look at #2670 and comment if you think something's off or missing? |
@edgarrmondragon took a look and looks good to me! |
@Mac-lp3 FWIW I think the docs change was super helpful! 😅 |
Thanks both! |
Copy of #2578 where I can push, since that one was created from a protected branch.
📚 Documentation preview 📚: https://meltano-sdk--2598.org.readthedocs.build/en/2598/