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

Remove add_related for TRANSFORMS #5957

Merged
merged 16 commits into from Jun 5, 2022
Merged

Conversation

kgpayne
Copy link
Contributor

@kgpayne kgpayne commented May 31, 2022

This pr:

  • removes the auto-add functionality that applied to Transforms (whereby the legacy 'dbt' Transformer was auto-added if it was missing)
  • fails if a Transform is added without a Transformer plugin present in the users Project

As a side effect, this now allows a Transform plugin to be added to a project with an adapter-specific dbt install. However using an adapter-specific dbt with existing Transforms will fail due to a missing DBT_SOURCE_SCHEMA env var that was present as a setting in the legacy 'dbt' Transformer plugin definition but is not included in the new adapter-specific Transformer definitions. This should be addressed in a future issue if we wish to support the use of Transforms with adapter-specific dbt Transformers.

@kgpayne kgpayne self-assigned this May 31, 2022
src/meltano/cli/utils.py Outdated Show resolved Hide resolved
Co-authored-by: Edgar R. M. <edgar@meltano.com>
@kgpayne kgpayne removed the request for review from pandemicsyn May 31, 2022 15:51
@kgpayne kgpayne changed the title Remove add_related for TRANSFORMS Draft: Remove add_related for TRANSFORMS May 31, 2022
@kgpayne kgpayne marked this pull request as draft May 31, 2022 15:52
@kgpayne kgpayne changed the title Draft: Remove add_related for TRANSFORMS Remove add_related for TRANSFORMS May 31, 2022
@aaronsteers
Copy link
Contributor

@kgpayne - This looks very close, with the one exception of some commented-out code snippet and a TODO marker.

What's left to do here and do we need to hand off or can you get the remainder complete?

@netlify
Copy link

netlify bot commented Jun 5, 2022

Deploy Preview for meltano canceled.

Name Link
🔨 Latest commit 2248ae9
🔍 Latest deploy log https://app.netlify.com/sites/meltano/deploys/629d002693aec7000906b09d

@kgpayne kgpayne marked this pull request as ready for review June 5, 2022 16:41
@kgpayne kgpayne requested review from a team and afolson as code owners June 5, 2022 16:41
@kgpayne kgpayne dismissed edgarrmondragon’s stale review June 5, 2022 16:51

Changes apply to outdated commit

@aaronsteers
Copy link
Contributor

I've resolved the merge conflict by re-locking poetry.

Comment on lines -50 to -54
@click.option(
"--include-related",
is_flag=True,
help="Also add transform plugins related to the identified discoverable extractor.",
)
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: We may want to call out in migration docs somewhere that the --include-related CLI arg is removed from meltano add.

Copy link
Contributor

@aaronsteers aaronsteers left a comment

Choose a reason for hiding this comment

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

Nice work, @kgpayne ! This looks ready to go.

@aaronsteers aaronsteers merged commit 55f37d2 into main Jun 5, 2022
@aaronsteers aaronsteers deleted the 3330-disable-auto-add-transformers branch June 5, 2022 19:34
@kgpayne kgpayne linked an issue Jun 8, 2022 that may be closed by this pull request
msardana94 added a commit to msardana94/meltano that referenced this pull request Apr 18, 2023
I believe `--include-related` is deprecated in 2.0.0 as mentioned in meltano#5957
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.

Fail adding transform plugin if no transformer is added
3 participants