-
Notifications
You must be signed in to change notification settings - Fork 71
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: enable Taps and Targets to generate their own Meltano yaml files #1094
Closed
aaronsteers
wants to merge
24
commits into
main
from
135-enable-generation-of-the-yaml-file-required-for-the-hub
Closed
Changes from 5 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
9f56e96
chore: initial refactor for readability
aaronsteers b85760a
feat: add Meltano rendering logic in private helper module
aaronsteers 8d95b22
feat: add `secret=True` support in JSON Schema type helpers
aaronsteers 2820b91
change: update examples to use 'secret=True' for protected settings
aaronsteers e74be3f
chore: flake8 fix
aaronsteers e0239b5
add unit tests for type helpers
aaronsteers 17905b1
fix missing secret flag on unit test
aaronsteers 9d6a364
chore: get tests passing
aaronsteers 8ad7727
chore: add test for description
aaronsteers 2792aa1
chore: remove commented code
aaronsteers b86cfc4
chore: remove files related to #1094
aaronsteers 6887faf
chore: dummy commit
aaronsteers 05edd84
Merge branch '77-feat-secrets-support-in-config-and-streams' into 135…
aaronsteers 2a87092
chore: add back files
aaronsteers 0bf2fb8
chore: revert dummy change
aaronsteers 2b83266
chore: revert --about updates
aaronsteers f8c734a
Merge branch 'main' into 77-feat-secrets-support-in-config-and-streams
aaronsteers 28be9cb
Merge branch '77-feat-secrets-support-in-config-and-streams' into 135…
aaronsteers 3cb21fc
chore: reapply updates
aaronsteers 3018690
move to new `singer-tools` helper CLI
aaronsteers 122e8eb
tweak cli defs
aaronsteers 1bfb60a
chore: remove stray file
aaronsteers 23aae72
chore: make file callable
aaronsteers 78d8aee
merge from origin/main
aaronsteers File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
"""Helper functions for Meltano and MeltanoHub interop.""" | ||
|
||
from __future__ import annotations | ||
|
||
from ._typing import ( | ||
is_array_type, | ||
is_boolean_type, | ||
is_datetime_type, | ||
is_integer_type, | ||
is_object_type, | ||
is_secret_type, | ||
is_string_type, | ||
) | ||
|
||
|
||
def _to_meltano_kind(jsonschema_type: dict) -> str | None: | ||
"""Returns a Meltano `kind` indicator for the provided JSON Schema type. | ||
|
||
For reference: | ||
https://docs.meltano.com/reference/plugin-definition-syntax#settingskind | ||
|
||
Args: | ||
jsonschema_type: JSON Schema type to check. | ||
|
||
Returns: | ||
A string representing the meltano 'kind'. | ||
""" | ||
if is_secret_type(jsonschema_type): | ||
return "password" | ||
|
||
if is_string_type(jsonschema_type): | ||
return "string" | ||
|
||
if is_object_type(jsonschema_type): | ||
return "object" | ||
|
||
if is_array_type(jsonschema_type): | ||
return "array" | ||
|
||
if is_boolean_type(jsonschema_type): | ||
return "boolean" | ||
|
||
if is_datetime_type(jsonschema_type): | ||
return "date_iso8601" | ||
|
||
if is_integer_type(jsonschema_type): | ||
return "integer" | ||
|
||
return None | ||
|
||
|
||
def meltano_yaml_str( | ||
plugin_name: str, | ||
capabilities: list[str], | ||
config_jsonschema: dict, | ||
) -> str: | ||
"""Returns a Meltano plugin definition as a yaml string. | ||
|
||
Args: | ||
plugin_name: Name of the plugin. | ||
capabilities: List of capabilities. | ||
config_jsonschema: JSON Schema of the expected config. | ||
|
||
Returns: | ||
A string representing the Meltano plugin Yaml definition. | ||
""" | ||
capabilities_str: str = "\n".join( | ||
[" - {capability}" for capability in capabilities] | ||
) | ||
settings_str: str = "\n".join( | ||
[ | ||
f""" | ||
- name: {setting_name} | ||
label: {setting_name.replace("_", " ").proper()} | ||
kind: {_to_meltano_kind(type_dict["type"])}, | ||
description: {type_dict.get("description", 'null')} | ||
""" | ||
for setting_name, type_dict in config_jsonschema["properties"].items() | ||
] | ||
) | ||
required_settings = [ | ||
setting_name | ||
for setting_name, type_dict in config_jsonschema.items() | ||
if setting_name in config_jsonschema.get("required", []) | ||
or type_dict.get("required", False) | ||
] | ||
settings_group_validation_str = " - - " + "\n - ".join(required_settings) | ||
|
||
return f""" | ||
name: {plugin_name} | ||
namespace: {plugin_name.replace('-', '_')} | ||
|
||
## The following could not be auto-detected: | ||
# maintenance_status: # | ||
# repo: # | ||
# variant: # | ||
# label: # | ||
# description: # | ||
# pip_url: # | ||
# domain_url: # | ||
# logo_url: # | ||
# keywords: [] # | ||
|
||
capabilities: | ||
{capabilities_str} | ||
settings_group_validation: | ||
{settings_group_validation_str} | ||
settings: | ||
{settings_str} | ||
""" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aaronsteers So, I guess this is done as pure string manipulation so comments can be added.
Wouldn't it make more sense for Meltano to ingest the output of
--about --format=json
to generate the config it expects? I say it because the SDK shouldn't need to know about what fields Meltano expects, but Meltano could have a utility CLI (bundled in the same Pythonmeltano
package perhaps) to parse the SDK plugin metadata:Another benefit of that approach is that developers (or us) don't need to upgrade their packages to be able to output the right Meltano config, rather users of Meltano bump to the latest
meltano
package to get this utility.Developers could still bump the SDK version in their packages if we update the metadata exposed through the
--about --format=json
output.Wydt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My intuition was similar at first, and certainly these are good points. Unfortunately, there just isn't a natural place for this to live in Meltano as of yet, and I think it will harder if we try to iterate both tools in lock step. I think we have an opportunity in the SDK to introduce a first round of interop, followed by quick patches if/when we find any issues. When stable and proven, we can migrate this functionality into Meltano - but I don't know that we yet have a good enough feature to make it worth adding to Meltano and then phasing out when we inevitably have a more robust paradigm. There just isn't anywhere I can imagine us putting this into Meltano as of today that wouldn't need to be removed/replaced in a future Meltano revision. But meanwhile, if we iterate in the SDK, we should get to a place where the issues are ironed out, and then we'll be closer to something like a
meltano publish extractor
ormeltano create extractor
type of a workflow.All that said, I really don't think there is any issue with baking MeltanoHub interop directly into the SDK, since it directly helps developers promote their taps, and likewise streamlines adoption/onboarding for users even if the tap is not on the Hub. It's not intrusive and doesn't overly promote or favor Meltano over other orchestrators.
It's possible we'll phase out
--about --format=meltano
before we get to 1.0, but I think it's a valuable and (relatively) stable increment from where we are today.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@edgarrmondragon - Since our conversation, I've refactored this to an internal/dev-side CLI.
This doesn't change the experience at all for tap users, but it makes a new command available to developers and power users who want to leverage it. As noted in
poetry.toml
, this can be executed from with the SDK repo during development, or by installingsinger-sdk
as a standalone program withpipx
.I don't think we should necessarily promote at this time, but developers can also execute this with
python -m singer_sdk.dev.cli --help
orpython -m singer_sdk.dev.cli analyze ...
anywhere that thesinger_sdk
library is installed.@WillDaSilva - In the long run, I wonder if we can+should put the hub analyze work here in the SDK
singer-tools
dev CLI. One of the reasons theanalyze
command stores its output to a directory of files is that I am imagining we could move that complex/plugin-test
logic here, and then let the Hub call this command - analyzing the tap or target and storing a bunch of file outputs based on the discovery and analysis.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aaronsteers my initial reactions were the same as @edgarrmondragon's in that we're tightly coupling the hub and the SDK/Meltano CLI by baking it in. Not that we're planning to change the hub definition structure much but if we ever did make a change then it means we'll most likely get user contributing invalid definitions because they're a version out of date.
What if we use the EDK to make a utility that converts the
--format=json
output to a hub yaml file. It's not necessarily where I was going with it originally but I created a utility that helps create taps/targets hub definitions and puts them in the right location in the hub repo. If we had a meltano extension that contained this logic then we're not tied to the SDK version or meltano version either, the user could runmeltano add utility hub-utils
thenmeltano invoke hub-utils add
and it kicks off prompts to get the--about
scrapped, prompt for any other info like a logo png, and write the yaml file to the appropriate directory in the hub repo. If we make any changes to the format or whatever, the user just needs to re-install the updated utility to get support for those changes.