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.

Sign up for GitHub

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

Jump to bottom

feat: Add Salesforce extractor #1370

Merged
merged 9 commits into from
Jul 23, 2021
Merged

feat: Add Salesforce extractor #1370

merged 9 commits into from
Jul 23, 2021

Conversation

benrifkind
Copy link
Contributor

Summary of Changes

This is an implementation of #1352. It adds a SalesForceExtractor which pulls metadata about SalesForce objects into a TableMetadata object.

Tests

tests/unit/extractor/test_salesforce_extractor.py

Documentation

Added to the README

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • PR includes a summary of changes.
  • PR adds unit tests, updates existing unit tests, OR documents why no test additions or modifications are needed.
  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does

@benrifkind
SalesForce extractor
e581fe5 
Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@benrifkind
Add simple-salesforce dependency
c6e6000 
Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@benrifkind
lint tests
a427fa4 
Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@benrifkind
Add license header
7e1fcbb 
Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@benrifkind
Add documentation
bdb4e77 
Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@boring-cyborg boring-cyborg bot added area:databuilder From databuilder folder area:all Related to all the project area:docs labels Jul 21, 2021
@benrifkind benrifkind changed the title feat:Add Salesforce extractor feat Add Salesforce extractor Jul 21, 2021
@benrifkind benrifkind changed the title feat Add Salesforce extractor feat: Add Salesforce extractor Jul 21, 2021
@benrifkind
Copy link
Contributor Author

benrifkind commented Jul 21, 2021 •
edited

Unit test failed because it doesn't have the simple-salesforce dependency. How do I add that in?

@benrifkind
Add salesforce to all_deps
e8af5a3 
Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@feng-tao
Copy link
Member

you could add a salesforce section in https://github.com/amundsen-io/amundsen/blob/main/databuilder/setup.py following other examples.

@benrifkind
Copy link
Contributor Author

I did that but I neglected to add it to all_deps which I just did.

@benrifkind
fix lint
89d708e 
Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@benrifkind
isort
85b790e 
Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@benrifkind
Copy link
Contributor Author

benrifkind commented Jul 21, 2021 •
edited

Linting is failing. I ran isort locally on the offending file but there are no changes. Possibly related - when I run mypy locally it complains about issues but the mypy check passes the check.
Edit: Figured it out. I was in the wrong python venv when running these checks.

@benrifkind
Actually fix issues
8073500 
Signed-off-by: benrifkind <ben.rifkind@gmail.com>
feng-tao
feng-tao reviewed Jul 22, 2021
View reviewed changes
Copy link
Member

@feng-tao feng-tao left a comment

Choose a reason for hiding this comment

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

lgtm overall with just one question

databuilder/databuilder/extractor/salesforce_extractor.py
# Filter the sobjects if `OBJECT_NAMES_KEY` is set otherwise return all
sobjects = [
sobject
for sobject in self._client.describe()["sobjects"]
Copy link
Member

Choose a reason for hiding this comment

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

is sobjects a typo?

Copy link
Member

Choose a reason for hiding this comment

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

or sobjects == salesforce objects...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sobjects is the key in the response so I just used it as the name but I could change it to salesforce_objects if you prefer.

@feng-tao feng-tao merged commit 15d1e83 into amundsen-io:main Jul 23, 2021
@benrifkind benrifkind deleted the salesforce-extractor branch July 23, 2021 01:46
zacr pushed a commit to SaltIO/amundsen that referenced this pull request May 13, 2022
@benrifkind
feat: Add Salesforce extractor (amundsen-io#1370)
318f054 
* SalesForce extractor

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* Add simple-salesforce dependency

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* lint tests

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* Add license header

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* Add documentation

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* Add salesforce to all_deps

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* fix lint

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* isort

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* Actually fix issues

Signed-off-by: benrifkind <ben.rifkind@gmail.com>
hansadriaans pushed a commit to DataChefHQ/amundsen that referenced this pull request Jun 30, 2022
@benrifkind
feat: Add Salesforce extractor (amundsen-io#1370)
52ea259 
* SalesForce extractor

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* Add simple-salesforce dependency

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* lint tests

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* Add license header

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* Add documentation

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* Add salesforce to all_deps

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* fix lint

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* isort

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* Actually fix issues

Signed-off-by: benrifkind <ben.rifkind@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feng-tao feng-tao feng-tao approved these changes

@allisonsuarez allisonsuarez Awaiting requested review from allisonsuarez allisonsuarez is a code owner

@dechoma dechoma Awaiting requested review from dechoma dechoma is a code owner

@dorianj dorianj Awaiting requested review from dorianj

@jinhyukchang jinhyukchang Awaiting requested review from jinhyukchang jinhyukchang is a code owner

@mgorsk1 mgorsk1 Awaiting requested review from mgorsk1 mgorsk1 is a code owner

@sewardgw sewardgw Awaiting requested review from sewardgw sewardgw is a code owner

@verdan verdan Awaiting requested review from verdan verdan is a code owner

@youngyjd youngyjd Awaiting requested review from youngyjd youngyjd is a code owner

Assignees
No one assigned
Labels
area:all Related to all the project area:databuilder From databuilder folder
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@benrifkind @feng-tao