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

Example has invalid JSON #797

Closed
visch opened this issue Jul 8, 2022 · 10 comments · Fixed by #1756
Closed

Example has invalid JSON #797

visch opened this issue Jul 8, 2022 · 10 comments · Fixed by #1756
Labels
Accepting Pull Requests Documentation Improvements or additions to documentation good first issue Good for newcomers kind/Bug Something isn't working

Comments

@visch
Copy link
Contributor

visch commented Jul 8, 2022

https://sdk.meltano.com/en/latest/stream_maps.html has invalid JSON

Specifically

{
    "stream_maps": {
        "customers": {
            // exclude these since we're capturing them in the pii stream
            "email": null,
            "full_name": null
        },
        "customers_pii": {
            "__source__": "customers",
            // include just the PII and the customer_id
            "customer_id": "customer_id",
            "email": "email",
            "full_name": "full_name",
            // exclude anything not declared
            "__else__": null,
        },
    },
}

Valid json would be

{
	"stream_maps": {
		"customers": {
			"email": null,
			"full_name": null
		},
		"customers_pii": {
			"__source__": "customers",
			"customer_id": "customer_id",
			"email": "email",
			"full_name": "full_name",
			"__else__": null
		}
	}
}
@edgarrmondragon edgarrmondragon added kind/Bug Something isn't working Documentation Improvements or additions to documentation Accepting Pull Requests labels Jul 15, 2022
@visch
Copy link
Contributor Author

visch commented Jul 17, 2022

Looked a fixing this an it's a slightly bigger question https://sdk.meltano.com/en/latest/stream_maps.html

On this page instead of the examples being JSON they are python dicts. What about using meltano.yml instead? Then we could add comments and be compliant. Seems like a much better place than we are. Issue is users that don't use meltano may be confused, so maybe for each example we have a meltano.yml , and a json equivalent?

That seems to make a lot of sense to me. Maybe a ui element like this https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/install-kubeadm/#:~:text=The%20tables%20below%20include%20the%20known%20endpoints%20for%20supported%20operating%20systems%3A . Default to meltano.yml ,and offer the json option so folks can copy paste into their config.json

@edgarrmondragon
Copy link
Collaborator

edgarrmondragon commented Jul 18, 2022

@visch I like the idea of having config examples both in meltano YAML format and raw JSON. I might give sphinx-tabs a try.

@edgarrmondragon
Copy link
Collaborator

edgarrmondragon commented Jul 18, 2022

I might give sphinx-tabs a try.

Bad luck, sphinx-tabs depends on docutils>=0.18 but sphinx-rtd-theme depends on docutils<0.18: readthedocs/sphinx_rtd_theme#1302

@edgarrmondragon
Copy link
Collaborator

We moved to Furo, so we should now be able to support sphinx-tabs, if anyone wants to tackle this

@echen805
Copy link

@edgarrmondragon if no one has called it internally, would love to try this over the weekend!

@edgarrmondragon
Copy link
Collaborator

@edgarrmondragon if no one has called it internally, would love to try this over the weekend!

@echen805 By all means, go for it!

@edgarrmondragon
Copy link
Collaborator

@echen805 Let me know if you're still interested in contributing 🙂

@edgarrmondragon edgarrmondragon added the good first issue Good for newcomers label May 24, 2023
@echen805
Copy link

Ooof sorry work has been keeping me real busy. You can un-assign me and I can help out when I have time again

@mjsqu
Copy link
Contributor

mjsqu commented Jun 8, 2023

I made a start on this here: https://github.com/mjsqu/sdk/tree/feat/sphinx_tabs

image

image

First time doing anything with sphinx so I thought I'd just make one update first to check it all looks ok on one of the code samples. I've shifted comments out of the JSON and into the meltano.yml tab.

@mjsqu
Copy link
Contributor

mjsqu commented Jun 8, 2023

Added tabs for all stream map samples to the same branch ⬆️

When using stream maps in meltano.yml is it ok to leave null out? I prefer specifying it to aid readability, so in the above example I think I corrected - e.g.:

# Exclude email
email: 

# to the following for more readability

# Explicitly use null to show email is not mapped
email: null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepting Pull Requests Documentation Improvements or additions to documentation good first issue Good for newcomers kind/Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants