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

Validation is added #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JennLeon
Copy link

@JennLeon JennLeon commented May 3, 2022

Validation is added to confirm that the requested key ['real_name'] exists, this is because it fails when there is no name field

…xists, this is because it fails when there is no name field
@halilduygulu
Copy link

If you have this problem and end up here from google search, you can add real_name to id column's metadata, which is not there when a catalog output generated.

{
  "breadcrumb": [
    "properties",
    "id"
  ],
  "metadata": {
    "inclusion": "automatic",
    "selected": true,
    "real_name": "id"
  }
}

Of course it would be really nice if this pr is merged and we would not have to pre-create catalog.json files to use this tap.

Comment on lines +193 to +194
if m["metadata"].get("real_name"):
return m["metadata"]["real_name"]
Copy link

Choose a reason for hiding this comment

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

Suggested change
if m["metadata"].get("real_name"):
return m["metadata"]["real_name"]
return m["metadata"].get("real_name", None)

@sonarcloud
Copy link

sonarcloud bot commented Oct 18, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link

@pedroceriotti pedroceriotti left a comment

Choose a reason for hiding this comment

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

This seems to fix the issue 👍

@pedroceriotti
Copy link

Any chance we can have this merged in? @lidalei @halilduygulu

I've seen other people on Meltano's Slack struggling with the same issue this PR resolves.

@halilduygulu
Copy link

Any chance we can have this merged in? @lidalei @halilduygulu

I am not sure who is repo owner or monitoring PRs.
We use commit id to install tap in our work. You can give any git sha to install taps, does not need to be master, like this

extractors:
  - name: tap-airtable
    variant: goes-funky
    pip_url: git+https://github.com/goes-funky/tap-airtable.git@xyz123

@pedroceriotti
Copy link

Any chance we can have this merged in? @lidalei @halilduygulu

I am not sure who is repo owner or monitoring PRs. We use commit id to install tap in our work. You can give any git sha to install taps, does not need to be master, like this

extractors:
  - name: tap-airtable
    variant: goes-funky
    pip_url: git+https://github.com/goes-funky/tap-airtable.git@xyz123

Got it, that's helpful!

However, this commit is behind the master branch, which brings some important recent updates for the tap to work properly.

Ideally, we'd always checkout from master to automatically get new update as they come too.

Anyway, thanks for your help!

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.

4 participants