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

🐛 Source Smartsheets: fix scrambled columns bug #11911

Merged
merged 6 commits into from
Apr 18, 2022

Conversation

davydov-d
Copy link
Collaborator

@davydov-d davydov-d commented Apr 12, 2022

What

#5520
#8099
image

[
  {'id': 1101932201830276, 'index': 0, 'title': 'id', 'type': 'TEXT_NUMBER'...},
  {'id': 5605531829200772, 'index': 1, 'title': 'first_name', 'type': 'TEXT_NUMBER'...}, 
  {'id': 3353732015515524, 'index': 2, 'title': 'last_name', 'type': 'TEXT_NUMBER'...}, 
  {'id': 7857331642886020, 'index': 3, 'title': 'email', 'type': 'TEXT_NUMBER'...}
]

Moving email column in the UI to the last position and performing read gives following results:

{"id": "99.0", "first_name": "Auria", "last_name": "Haryngton", "gender": "aharyngton2q@mapquest.com", "ip_address": "Non-binary", "dob": "246.28.159.95", "email": "2020-06-28"}

How

  1. Do not take columns from Stream instance, use Sheet instance instead
  2. Map cells and columns by columnId
  3. Add unit test
  4. Fix integration test
  5. Speed up Dockerfile (perform COPY after pip install)

@github-actions github-actions bot added the area/connectors Connector related issues label Apr 12, 2022
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Apr 12, 2022
@codecov
Copy link

codecov bot commented Apr 12, 2022

Codecov Report

❗ No coverage uploaded for pull request base (master@fa9c467). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #11911   +/-   ##
=========================================
  Coverage          ?   45.20%           
=========================================
  Files             ?        2           
  Lines             ?       73           
  Branches          ?        0           
=========================================
  Hits              ?       33           
  Misses            ?       40           
  Partials          ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa9c467...ef45264. Read the comment docs.

@davydov-d
Copy link
Collaborator Author

davydov-d commented Apr 12, 2022

/test connnector=connectors/source-smartsheets

Error: Unexpected inputs provided: ["connnector"]

@davydov-d
Copy link
Collaborator Author

davydov-d commented Apr 12, 2022

/test connector=connectors/source-smartsheets

🕑 connectors/source-smartsheets https://github.com/airbytehq/airbyte/actions/runs/2153413712
✅ connectors/source-smartsheets https://github.com/airbytehq/airbyte/actions/runs/2153413712
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              285    106    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  886    259    71%
Name                             Stmts   Miss  Cover
----------------------------------------------------
source_smartsheets/__init__.py       2      0   100%
source_smartsheets/source.py        71     40    44%
----------------------------------------------------
TOTAL                               73     40    45%

Python short test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.7/site-packages/source_acceptance_test/plugin.py:56: Skipping TestIncremental.test_two_sequential_reads because not found in the config
======================== 20 passed, 1 skipped in 8.85s =========================

@davydov-d
Copy link
Collaborator Author

davydov-d commented Apr 14, 2022

/test connector=connectors/source-smartsheets

🕑 connectors/source-smartsheets https://github.com/airbytehq/airbyte/actions/runs/2165921820
✅ connectors/source-smartsheets https://github.com/airbytehq/airbyte/actions/runs/2165921820
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              285    106    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  886    259    71%
Name                             Stmts   Miss  Cover
----------------------------------------------------
source_smartsheets/__init__.py       2      0   100%
source_smartsheets/source.py        71     40    44%
----------------------------------------------------
TOTAL                               73     40    45%

Python short test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.7/site-packages/source_acceptance_test/plugin.py:56: Skipping TestIncremental.test_two_sequential_reads because not found in the config
======================== 20 passed, 1 skipped in 9.18s =========================

@davydov-d
Copy link
Collaborator Author

davydov-d commented Apr 18, 2022

/test connector=connectors/source-smartsheets

🕑 connectors/source-smartsheets https://github.com/airbytehq/airbyte/actions/runs/2184214471
✅ connectors/source-smartsheets https://github.com/airbytehq/airbyte/actions/runs/2184214471
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              285    106    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  886    259    71%
Name                             Stmts   Miss  Cover
----------------------------------------------------
source_smartsheets/__init__.py       2      0   100%
source_smartsheets/source.py        71     40    44%
----------------------------------------------------
TOTAL                               73     40    45%

Python short test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.7/site-packages/source_acceptance_test/plugin.py:56: Skipping TestIncremental.test_two_sequential_reads because not found in the config
======================== 20 passed, 1 skipped in 9.56s =========================

Copy link
Contributor

@midavadim midavadim left a comment

Choose a reason for hiding this comment

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

This update can be released without Airbyte review

@davydov-d
Copy link
Collaborator Author

davydov-d commented Apr 18, 2022

/publish connector=connectors/source-smartsheets

🕑 connectors/source-smartsheets https://github.com/airbytehq/airbyte/actions/runs/2184811347
🚀 Successfully published connectors/source-smartsheets
🚀 Auto-bumped version for connectors/source-smartsheets
✅ connectors/source-smartsheets https://github.com/airbytehq/airbyte/actions/runs/2184811347

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets April 18, 2022 16:30 Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets April 18, 2022 16:30 Inactive
@davydov-d davydov-d merged commit 5c5f38a into master Apr 18, 2022
@davydov-d davydov-d deleted the ddavydov/5520-source-smartsheets-columns-scrambled branch April 18, 2022 17:08
suhomud pushed a commit that referenced this pull request May 23, 2022
* #5520 fix scrambled columns bug

* #5520 source smartsheets: add changelog item

* #5520 move pytest to optional setup requirements

* #5520 source smartsheets: add two unit tests

* auto-bump connector version

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Smartsheets connector raw key:value pair data is scrambled. Smart Sheet Conector / Columns Scrambled
6 participants