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

Add tap-netsuite variant #1403

Merged
merged 2 commits into from Jun 27, 2023
Merged

Add tap-netsuite variant #1403

merged 2 commits into from Jun 27, 2023

Conversation

sehnem
Copy link
Contributor

@sehnem sehnem commented Jun 27, 2023

Checklist

  • Add/update the file in the appropriate folder (/taps or /targets). The name of the file should match the name of the tap. If there is already one, add a descriptor to the name such as -search.
  • Add/update the PNG logo image in /assets/logos/<taps or targets>. The image name must match the YAML file name.
  • Tag @tayloramurphy or @pnadolny13 to flag it for review. Or post to the #hub channel on Meltano slack.

Reviewer Checklist

@netlify
Copy link

netlify bot commented Jun 27, 2023

👷 Deploy request for meltano-hub pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 491fcf1

@tayloramurphy
Copy link
Collaborator

Exciting to see! @pnadolny13 will you review and merge?

Copy link
Contributor

@pnadolny13 pnadolny13 left a comment

Choose a reason for hiding this comment

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

@sehnem this is awesome!

Relative to the other variants on the hub like the gary james variant https://hub.meltano.com/extractors/tap-netsuite--gthesheep/, it looks like this uses the SOAP API while the other one uses REST. Is that true? I added a optional suggestion to add SOAP to the label so people quickly see the difference.

What do you think?

@sehnem
Copy link
Contributor Author

sehnem commented Jun 27, 2023

@pnadolny13 I agree, just didn't want to change the current name, but it would make things much clearer, given that there are three versions of NetSuite API, SOAP, REST and SuiteQL, and there is also a difference on the schema of the streams, so we cannot just replace SOAP with REST.

@sehnem
Copy link
Contributor Author

sehnem commented Jun 27, 2023

I also set the quality to silver, I know that it is a new tap, but I think that overall it is more organized than the others and is using the SDK, although it need to be tested with more accounts. Let me know if you think it would be better to set it to bronze.

@pnadolny13
Copy link
Contributor

@pnadolny13 I agree, just didn't want to change the current name, but it would make things much clearer, given that there are three versions of NetSuite API, SOAP, REST and SuiteQL, and there is also a difference on the schema of the streams, so we cannot just replace SOAP with REST.

This is a good point. Its a little weird to have a default variant in this case. I'm not very familiar with Netsuite, are the REST/SOAP/SuiteQL taps all attempting to extract somewhat similar data? Or is there totally different data coming from each different API.

It makes me wonder if we should split these out to their own standalone tap pages, we'd basically rename this tap on the hub to tap-netsuite-soap. We should merge this for now but if you think that makes sense I can open an issue to split them out eventually.

@sehnem
Copy link
Contributor Author

sehnem commented Jun 27, 2023

This is a good point. Its a little weird to have a default variant in this case. I'm not very familiar with Netsuite, are the REST/SOAP/SuiteQL taps all attempting to extract somewhat similar data? Or is there totally different data coming from each different API.

It makes me wonder if we should split these out to their own standalone tap pages, we'd basically rename this tap on the hub to tap-netsuite-soap. We should merge this for now but if you think that makes sense I can open an issue to split them out eventually.

The data is the same across different versions. But, for example, in SuiteQL, both creditMemo and invoice are part of the transaction object, in the SOAP and REST versions, they are standalone objects. The SOAP and REST objects have mostly the same structure. The SOAP version being the most complete and stable one, while the REST version has some endpoints that are still in beta but accept newer authentication methods. SuiteQL is mostly used for custom reports and similar tasks.
As the data is the same I think that they can be in the same page, and the user can choose based on the usage. The SOAP version should cover most of the use cases.

@pnadolny13 pnadolny13 merged commit c9a4b48 into meltano:main Jun 27, 2023
7 checks passed
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.

None yet

3 participants