meltano / sdk

Write 70% less code by using the SDK to build custom extractors and loaders that adhere to the Singer standard: https://sdk.meltano.com
https://sdk.meltano.com
Apache License 2.0
87 stars 64 forks source link

Add settings 'write-back' capability (Singer SDK) #106

Open MeltyBot opened 3 years ago

MeltyBot commented 3 years ago

Migrated from GitLab: https://gitlab.com/meltano/sdk/-/issues/106

Originally created by @aaronsteers on 2021-04-15 21:08:05


Related to similar issue for Meltano: https://gitlab.com/meltano/meltano/-/issues/2710

Some tap and target implementations use a writeback method (we think?) to store settings (such as auth tokens) back in the config.json file for future use.

There's currently no similar way for a tap developer using the SDK to write back settings for use in future invocations.

Wanted:

For this issue, we are currently gaging feedback and asking for developers to provide use cases if this is a blocker for them. If this affects you, please post to the comments.

Related

MeltyBot commented 2 years ago

View 2 previous comments from the original issue on GitLab

stale[bot] commented 11 months ago

This has been marked as stale because it is unassigned, and has not had recent activity. It will be closed after 21 days if no further activity occurs. If this should never go stale, please add the evergreen label, or request that it be added.

pnadolny13 commented 3 months ago

@edgarrmondragon I implemented a version of this with the SDK in tap-gohighlevel.

I honestly didnt spend a lot of time thinking through the cleanest way to pass this stuff around so let me know if theres a better approach.

edgarrmondragon commented 3 months ago

Thanks for sharing the implementation @pnadolny13!

  • I had to override the tap init to capture the config file path and save it to the tap instance. I also chose to raise an exception if the configs are passed in outside of a config file.

Yeah, I hadn't thought about it but more multiple --config values or --config=ENV don't really work with write-back, and write-back is a hard requirement for tap so it makes sense to straight-out fail.

That's a not a problem for Meltano because it always uses a (single) config file under the hood, and I'm willing to bet close to no one uses multiple --config flags.

That implementation does not seem to handle cases where --config=ENV is used to pass the refresh token, though.

  • Then when my authenticator is being instantiated by pull the config path variable out and pass it in. The way I access that var is definitely not nice but it works.

Cool! Though I don't particularly love create_for_stream cause in general it does not provide any benefits over a regular __init__ but it couples the stream and authenticator classes in a circular manner 😅.

  • I overrode update_access_token to catch the new refresh token
  • Then call a method _write_back_to_config that overwrites the file.

This combination makes me want to add a hook somewhere in the SDK that lets developers send a signal that the input config file should be updated.


I think the implementation makes sense, since it works well with Meltano's standard config location (as opposed to writing back to an arbitrary file) and it gives a reference implementation to adapt upstream here.

Unfortunately https://github.com/meltano/meltano/issues/2660 is certainly harder to reason about and implement, but we can get something out for the SDK with this as a start.