feat: Permit mapper plugins as valid blocks #9105

ReubenFrankel · 2025-02-24T15:56:11Z

Description

Removes the

block violates set requirements: Expected unique mappings name not the mapper plugin name:

error message to optimistically allow mappers to be invoked directly, without having to instead reference a mapping.

As far as I see it, a mapping is just aliased config for mapper plugins. Some mappers will only ever need to perform a single function, so it doesn't makes sense to define a mapping in that case - you may as well just invoke the mapper directly; which is what this PR is addressing. For meltano-map-transformer and similar, I believe there is still value in mappings as it prevents you having to inherit_from a base meltano-map-transformer to define your reusable mappings (although I think they are functionally the same) - i.e.

plugins:
  mappers:
  - name: meltano-map-transformer
    variant: meltano
    pip_url: meltano-map-transform
    mappings:
    - name: all-caps
      config:
        stream_maps:
          '*':
            __alias__: __stream_name__.upper()
    - name: no-caps
      config:
        stream_maps:
          '*':
            __alias__: __stream_name__.lower()

and

plugins:
  mappers:
  - name: meltano-map-transformer
    variant: meltano
    pip_url: meltano-map-transform
  - name: all-caps
    inherit_from: meltano-map-transformer
    config:
      stream_maps:
        '*':
          __alias__: __stream_name__.upper()
  - name: no-caps
    inherit_from: meltano-map-transformer
    config:
      stream_maps:
        '*':
          __alias__: __stream_name__.lower()

Related Issues

Partially addresses Meltano Mappers and Mappings Discussion Issue #3233
Related to feat: Add support for mappers in the invoke command #7992
Related GitLab issues:

netlify · 2025-02-24T15:56:15Z

✅ Deploy Preview for meltano ready!

Name	Link
🔨 Latest commit	`6f4f5d7`
🔍 Latest deploy log	https://app.netlify.com/projects/meltano/deploys/68b639ea694dac0008181e55
😎 Deploy Preview	https://deploy-preview-9105--meltano.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

ReubenFrankel · 2025-02-26T14:27:33Z

@edgarrmondragon Should we just remove the portion of the failing test_run_mapper_config test that checks for an exception on mapper/mapping collision (https://github.com/meltano/meltano/actions/runs/13532529659/job/37817760752?pr=9105#step:12:2257), or is there something better we can do?

edgarrmondragon · 2025-02-26T15:48:34Z

@edgarrmondragon Should we just remove the portion of the failing test_run_mapper_config test that checks for an exception on mapper/mapping collision (https://github.com/meltano/meltano/actions/runs/13532529659/job/37817760752?pr=9105#step:12:2257), or is there something better we can do?

We probably just need to remove that test.

…o mappings is invoked by the plugin name

… name is invoked by the ambiguous plugin/mapping name

ReubenFrankel · 2025-02-27T13:06:20Z

@edgarrmondragon Should we just remove the portion of the failing test_run_mapper_config test that checks for an exception on mapper/mapping collision (https://github.com/meltano/meltano/actions/runs/13532529659/job/37817760752?pr=9105#step:12:2257), or is there something better we can do?

We probably just need to remove that test.

I removed the part of the test in 96fd5a4. I updated a separate part in d4047a9 to assert an AmbiguousMappingName error raised when a mapper plugin name and mapping name conflict, but couldn't find where to apply the change to meet that expectation. Unless you were talking about removing the whole test?

edgarrmondragon · 2025-03-13T22:04:06Z

Unless you were talking about removing the whole test?

I think this is definitely easier, so I'm OK with taking that path.

for more information, see https://pre-commit.ci

codecov · 2025-03-13T23:10:14Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.47%. Comparing base (9a726a2) to head (6f4f5d7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9105   +/-   ##
=======================================
  Coverage   95.47%   95.47%           
=======================================
  Files         259      259           
  Lines       21374    21388   +14     
  Branches     1291     1293    +2     
=======================================
+ Hits        20406    20420   +14     
  Misses        775      775           
  Partials      193      193

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

edgarrmondragon · 2025-03-19T14:03:06Z

Trying to think what unexpected behavior this might cause for users, but can't think of any 🤔

ReubenFrankel · 2025-03-19T14:11:32Z

Trying to think what unexpected behavior this might cause for users, but can't think of any 🤔

There is no check for conflicts between mapper plugin name and mapping name. I don't actually know which one it is resolving here:

meltano.yml

  - name: mapper-fivetran
    variant: matatika
    pip_url: git+https://github.com/Matatika/mapper-fivetran.git
    mappings:
    - name: mapper-fivetran

meltano invoke mapper-fivetran
meltano run <tap> mapper-fivetran <target>

This is what I was talking about above:

I updated a separate part in d4047a9 to assert an AmbiguousMappingName error raised when a mapper plugin name and mapping name conflict, but couldn't find where to apply the change to meet that expectation

edgarrmondragon

The mapper-level config doesn't seem to be used and instead the last found mapping's is used 🤔

Permit mapper plugins as valid blocks

f9ac7bd

ReubenFrankel requested review from WillDaSilva and edgarrmondragon as code owners February 24, 2025 15:56

Merge branch 'main' into meltano-run-mapper

2b13c34

ReubenFrankel changed the title ~~Permit mapper plugins as valid blocks~~ feat: Permit mapper plugins as valid blocks Feb 24, 2025

edgarrmondragon assigned edgarrmondragon and ReubenFrankel Feb 25, 2025

edgarrmondragon added Awaiting Action/Author Community-Contributed PR labels Feb 25, 2025

Merge branch 'main' into meltano-run-mapper

de989cd

ReubenFrankel added 2 commits February 27, 2025 12:41

Remove part of test that expects an error when a mapper plugin with n…

96fd5a4

…o mappings is invoked by the plugin name

Update expected error when a mapper plugin with a mapping of the same…

d4047a9

… name is invoked by the ambiguous plugin/mapping name

ReubenFrankel added 2 commits February 27, 2025 13:06

Merge branch 'main' into meltano-run-mapper

38ffaf5

Merge branch 'main' into meltano-run-mapper

58ec298

ReubenFrankel and others added 2 commits March 13, 2025 22:50

Remove test_run_mapper_config entirely

74e37c5

[pre-commit.ci] auto fixes from pre-commit.ci hooks

c40f1be

for more information, see https://pre-commit.ci

Merge branch 'main' into meltano-run-mapper

a913ed3

edgarrmondragon removed the Awaiting Action/Author label Mar 19, 2025

edgarrmondragon unassigned ReubenFrankel Mar 19, 2025

Merge branch 'main' into meltano-run-mapper

3a0f0a6

edgarrmondragon added the plugin management label Jun 20, 2025

Merge branch 'main' into meltano-run-mapper

78f4e14

ReubenFrankel mentioned this pull request Aug 5, 2025

feature: Support <PLUGIN_NAME>_<SETTING_NAME> convention for Mapper plugins #6811

Open

Merge branch 'main' into meltano-run-mapper

18beee6

tayloramurphy removed the request for review from WillDaSilva August 7, 2025 01:20

Merge branch 'main' into meltano-run-mapper

a4d86e4

edgarrmondragon force-pushed the main branch from 5c19764 to d7dc7ba Compare August 26, 2025 21:54

edgarrmondragon added 4 commits August 27, 2025 13:21

Merge branch 'main' into meltano-run-mapper

aff1375

Check ambiguous mapping and mapper name

18e15e5

Merge branch 'main' into meltano-run-mapper

ed28648

Split exception classes

6f4f5d7

edgarrmondragon approved these changes Sep 2, 2025

View reviewed changes

edgarrmondragon requested changes Sep 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Permit mapper plugins as valid blocks #9105

feat: Permit mapper plugins as valid blocks #9105

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat: Permit mapper plugins as valid blocks #9105

Are you sure you want to change the base?

feat: Permit mapper plugins as valid blocks #9105

Uh oh!

Conversation

Uh oh!

Description

Related Issues

Uh oh!

Uh oh!

✅ Deploy Preview for meltano ready!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants