feat(mappers): Stream map expressions now have access to the Faker class, rather than just a faker instance

[edgarrmondragon]

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/

[codspeed-hq]

CodSpeed Performance Report

Merging #2598 will not alter performance

_{Comparing 2578-copy (142d091) with main (be349ac)}

Summary

✅ 6 untouched benchmarks

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.50%. Comparing base (22d4eae) to head (2dec594).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2598   +/-   ##
=======================================
  Coverage   89.50%   89.50%           
=======================================
  Files          58       58           
  Lines        4804     4804           
  Branches      943      943           
=======================================
  Hits         4300     4300           
  Misses        351      351           
  Partials      153      153           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ReubenFrankel]

Maybe I'm missing something, but could this not have been achieved already using seed_instance?

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

[edgarrmondragon]

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?

[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: ***@***.***>

[Mac-lp3]

@ReubenFrankel you are 100% correct, you can accomplish the exact same thing with fake.seed_instance(), so adding Fake was not necessary 🤦

@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 fake.seed_instance() rather than Fake.seed(), which I don't mind fixing since its my mistake...

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 mapper.py?

[edgarrmondragon]

@ReubenFrankel you are 100% correct, you can accomplish the exact same thing with fake.seed_instance(), so adding Fake was not necessary 🤦

@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 fake.seed_instance() rather than Fake.seed(), which I don't mind fixing since its my mistake...

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 mapper.py?

Thanks for the offer @Mac-lp3! Can you take a look at #2670 and comment if you think something's off or missing?

[Mac-lp3]

@edgarrmondragon took a look and looks good to me!

[ReubenFrankel]

@Mac-lp3 FWIW I think the docs change was super helpful! 😅

[edgarrmondragon]

Thanks both!