< aaronsteers> I hope it s ok to ping you directly I m worki Meltano #plugins-general

<@U06CBKL5W7L> I hope it's ok to ping you directly...

laurent

09/09/2021, 9:05 PM

@aaronsteers I hope it's ok to ping you directly. I'm working on the PR for

tap-github

to add the repo list, and I think I caught a bug in the sdk, but wanted to check with you if I'm using it correctly:

Copy code

tap1 = TapGitHub(config=REPO_LIST_CONFIG)
tap1.run_discovery()
catalog = tap1.catalog_dict
set_catalog_stream_selected(catalog, "issue_comments", selected=False)
# now I expect catalog to contain "selected": false for the issue_comments stream, but it doesn't
tap2 = TapGitHub(config=REPO_LIST_CONFIG, catalog=catalog)
tap2.sync_all()
# I would expect tap2 to NOT sync the issue_comments stream

set_catalog_stream_selected

seems to modify a copy of a part of the original catalog, and not the object passed, meaning it seems to have no effect. Is my code above correct (or is the bug in here)?

laurent

09/09/2021, 9:11 PM

meh, I just saw #C01PKLU5D1R I should have posted there instead, sorry!

aaronsteers

09/09/2021, 9:36 PM

In general, yes, posting to #C01PKLU5D1R is preferred for general visibility, but yes, I see what you mean. One second though while I check something...

aaronsteers

09/09/2021, 9:38 PM

@laurent - yes, I believe this is fixed in the release coming out later today. https://gitlab.com/meltano/sdk/-/merge_requests/146

aaronsteers

09/09/2021, 9:39 PM

At least, I can see that this was refactored/rewritten.

aaronsteers

09/09/2021, 9:41 PM

All of the helpers modules prefixed with

are somewhat more raw and untested, hence the private module naming convention. In general, you should avoid calling those directly because they may be removed/refactored without warning. However, with @edgar_ramirez_mondragon's latest update, we agreed in that MR that we should start moving towards what the public and stable method signatures should be.

laurent

09/09/2021, 9:44 PM

Nice! some of these helper functions are super helpful for testing taps. I was just looking at

deselect_all_streams

for instance. I think exposing some of them at least for that purpose would be nice.

aaronsteers

09/09/2021, 9:44 PM

Here's the logged issue to try to come up with stable methods for devs to use: Consider promoting the Singer classes added in !146 to a public module (#209) · Issues · Meltano / Meltano SDK for Singer Taps and Targets · GitLab

aaronsteers

09/09/2021, 9:45 PM

Totally! Yes, we would like to. I used the same in one of my tests in the sample gitlab tap - which is a sign that it's needed and probably should be made non-private as soon as we can.

aaronsteers

09/09/2021, 9:46 PM

That said, the release that comes out today will break that existing method call, while introducing hopefully a better one.

aaronsteers

09/09/2021, 9:46 PM

If you want to add comments or suggestions on #209, that would be much appreciated.

laurent

09/09/2021, 9:48 PM

ok, I guess I'll just hold back on that PR for a few hours then 🙂

Open in Slack

Previous Next