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

Tdl 23543 fix uuid transformation #100

Merged

Conversation

RushiT0122
Copy link
Contributor

@RushiT0122 RushiT0122 commented Jul 18, 2023

Description of change

  • Fix the uuid transformation issue because UUIDLegacy support was removed by PyMongo v4.3+ and reads UUID fields as bson.binary.Binary object causes the transform issue.
  • Fix multiple compatibility issues in integration tests.

QA steps

  • automated tests passing

On local env, tested compatibility with,

  • MongoDB v4.4
  • MongoDB v.5.0
  • MongoDB v6.0

Risks

Rollback steps

  • revert this branch

@RushiT0122 RushiT0122 force-pushed the TDL-23543-fix-uuid-transformation branch from 3d904b8 to 22ac45c Compare July 18, 2023 14:38
@RushiT0122 RushiT0122 changed the base branch from master to TDL-22154-upgrade-pymongo July 18, 2023 16:00
@RushiT0122 RushiT0122 changed the base branch from TDL-22154-upgrade-pymongo to master July 18, 2023 17:16
@RushiT0122
Copy link
Contributor Author

Since Pymongo v4.4.0 is incompatible with python version 3.5.6 currently CCi is failing. We will be able to run it end-2-end once we merge Orchestrator PR#292.

@RushiT0122 RushiT0122 changed the title Tdl 23543 fix UUID transformation Tdl 23543 fix uuid transformation Jul 18, 2023
@RushiT0122 RushiT0122 changed the base branch from master to TDL-22154-upgrade-pymongo July 18, 2023 18:30
@@ -306,3 +330,8 @@ def test_run(self):

# validate that the state value has not changed after deleting the bookmarked value in each collection
self.assertEqual(state_2, state_3)

# Reset the transaction life time limit
LOGGER.info(f"Re-setting transaction life time limit to {original_life_time_limit} seconds")
Copy link
Contributor

@bhtowles bhtowles Jul 20, 2023

Choose a reason for hiding this comment

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

I've been informed that using f strings in LOGGER will fail pylint so I've been using:
LOGGER.info("Text %s", variable) format now

Copy link
Contributor Author

@RushiT0122 RushiT0122 Jul 20, 2023

Choose a reason for hiding this comment

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

I have not seen this issue. Also AFAIK pylint doesn't run on tests directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, it only runs on ui-automation / webapp side. But I try to be consistent across both

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the f-string and replaced it with .format().

Makefile Outdated
@@ -1,4 +1,4 @@
.DEFAULT_GOAL := test

test:
pylint tap_mongodb tap_mongodb/sync_strategies -d missing-docstring,fixme,duplicate-code,line-too-long,too-many-statements,too-many-locals,consider-using-f-string,consider-using-from-import
pylint tap_mongodb tap_mongodb/sync_strategies -d missing-docstring,fixme,duplicate-code,line-too-long,too-many-statements,too-many-locals,consider-using-f-string,consider-using-from-import,,broad-exception-raised,superfluous-parens,consider-using-generator
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there is an extra comma here. I'm a little shocked pylint doesn't complain about this 😂

@@ -1,5 +1,6 @@
import os
import pymongo
from tap_tester.logger import LOGGER
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
from tap_tester.logger import LOGGER
from tap_tester.logger import LOGGER

# Reset the transaction life time limit
LOGGER.info("Re-setting transaction life time limit to {} seconds".format(original_life_time_limit))
success, _ = self.set_transaction_life_time_limit_seconds(client, original_life_time_limit)
self.assertTrue(success, msg="Failed to reset transaction life time limit")
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to fail the test if all test assertions pass, but the clean up fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are making changes to the database-level configuration. If we don't reset it then the remaining integration tests will run with the updated configuration, potentially causing us to miss critical issues.

Copy link
Contributor

@luandy64 luandy64 left a comment

Choose a reason for hiding this comment

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

Approving assuming the tests pass

shantanu73 and others added 3 commits July 26, 2023 11:47
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.
- sort the dictionary before assertion
RushiT0122 and others added 17 commits July 26, 2023 11:47
- improve integration testlogging
…-io/tap-mongodb into TDL-23543-fix-uuid-transformation
* Added CircleCI config for mongodb 6.0.

* Added Ciall testsoin CCI for ngodb 6.5.0 & 6.0

* Fixed CCI config and added all tests for mongodb 4.4

* Removed mongodb 4.2 testing from CCi as it has reached end of life.

* Fixed redundant statement to include all integration tests.

* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* - set maxDiff to None to reset dict comparison limit
- sort the dictionary before assertion

* fix uuid transformation

* upgrade pymongo version in cci configuration

* fix cci configuration

* upgrade pip and setuptools

* update cci config to run on latest python version

* fix linting issues

* fix uuid bookmarking

* fix integration tests

* remove debugger import

* fix integration test compatibility issues with pymongo v4.3+ and mongoddb v6.0

* - fix transaction life time limit
- improve integration testlogging

* Added CircleCI config for mongodb 6.0.

* Added Ciall testsoin CCI for ngodb 6.5.0 & 6.0

* Fixed CCI config and added all tests for mongodb 4.4

* Removed mongodb 4.2 testing from CCi as it has reached end of life.

* Fixed redundant statement to include all integration tests.

* Removed glob & for loop statements as all integration tests are running in CCi.

---------

Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.com>
* projection test change

* - fix the emtpy projection issue
- fix the integration tests

* minor integration test fixes

* remove redundant comments

* Force {} projection to be None

* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* - set maxDiff to None to reset dict comparison limit
- sort the dictionary before assertion

* fix uuid transformation

* upgrade pymongo version in cci configuration

* fix cci configuration

* upgrade pip and setuptools

* update cci config to run on latest python version

* fix linting issues

* fix uuid bookmarking

* fix integration tests

* remove debugger import

* fix integration test compatibility issues with pymongo v4.3+ and mongoddb v6.0

* - fix transaction life time limit
- improve integration testlogging

* projection test change

* - fix the emtpy projection issue
- fix the integration tests

* minor integration test fixes

* remove redundant comments

* fix typo

* remove f-string from logger statements

* Force {} projection to be None

---------

Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.com>
Co-authored-by: shantanu73 <shantanudhiman25@gmail.com>
@shantanu73 shantanu73 merged commit c900ebb into TDL-22154-upgrade-pymongo Jul 27, 2023
3 checks passed
shantanu73 added a commit that referenced this pull request Jul 27, 2023
* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* Tdl 23543 fix uuid transformation (#100)

* - set maxDiff to None to reset dict comparison limit
- sort the dictionary before assertion

* fix uuid transformation

* upgrade pymongo version in cci configuration

* fix cci configuration

* upgrade pip and setuptools

* update cci config to run on latest python version

* fix linting issues

* fix uuid bookmarking

* fix integration tests

* remove debugger import

* fix integration test compatibility issues with pymongo v4.3+ and mongoddb v6.0

* - fix transaction life time limit
- improve integration testlogging

* fix typo

* remove f-string from logger statements

* minor code cleanup

* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* - set maxDiff to None to reset dict comparison limit
- sort the dictionary before assertion

* fix uuid transformation

* upgrade pymongo version in cci configuration

* fix cci configuration

* upgrade pip and setuptools

* update cci config to run on latest python version

* fix linting issues

* fix uuid bookmarking

* fix integration tests

* remove debugger import

* fix integration test compatibility issues with pymongo v4.3+ and mongoddb v6.0

* - fix transaction life time limit
- improve integration testlogging

* fix typo

* remove f-string from logger statements

* minor code cleanup

* [Tdl 23471] Fix circle ci config (#101)

* Added CircleCI config for mongodb 6.0.

* Added Ciall testsoin CCI for ngodb 6.5.0 & 6.0

* Fixed CCI config and added all tests for mongodb 4.4

* Removed mongodb 4.2 testing from CCi as it has reached end of life.

* Fixed redundant statement to include all integration tests.

* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* - set maxDiff to None to reset dict comparison limit
- sort the dictionary before assertion

* fix uuid transformation

* upgrade pymongo version in cci configuration

* fix cci configuration

* upgrade pip and setuptools

* update cci config to run on latest python version

* fix linting issues

* fix uuid bookmarking

* fix integration tests

* remove debugger import

* fix integration test compatibility issues with pymongo v4.3+ and mongoddb v6.0

* - fix transaction life time limit
- improve integration testlogging

* Added CircleCI config for mongodb 6.0.

* Added Ciall testsoin CCI for ngodb 6.5.0 & 6.0

* Fixed CCI config and added all tests for mongodb 4.4

* Removed mongodb 4.2 testing from CCi as it has reached end of life.

* Fixed redundant statement to include all integration tests.

* Removed glob & for loop statements as all integration tests are running in CCi.

---------

Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.com>

* Tdl 23609 fix empty projection (#102)

* projection test change

* - fix the emtpy projection issue
- fix the integration tests

* minor integration test fixes

* remove redundant comments

* Force {} projection to be None

* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* - set maxDiff to None to reset dict comparison limit
- sort the dictionary before assertion

* fix uuid transformation

* upgrade pymongo version in cci configuration

* fix cci configuration

* upgrade pip and setuptools

* update cci config to run on latest python version

* fix linting issues

* fix uuid bookmarking

* fix integration tests

* remove debugger import

* fix integration test compatibility issues with pymongo v4.3+ and mongoddb v6.0

* - fix transaction life time limit
- improve integration testlogging

* projection test change

* - fix the emtpy projection issue
- fix the integration tests

* minor integration test fixes

* remove redundant comments

* fix typo

* remove f-string from logger statements

* Force {} projection to be None

---------

Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.com>
Co-authored-by: shantanu73 <shantanudhiman25@gmail.com>

---------

Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.com>
Co-authored-by: shantanu73 <shantanudhiman25@gmail.com>

* renamed build_mongo_4_4 to build in config.yml

* Renamed build_mongo_4_4 to build in config.yml

---------

Co-authored-by: Rushikesh Todkar <98420315+RushiT0122@users.noreply.github.com>
Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.com>
shantanu73 added a commit that referenced this pull request Jul 27, 2023
* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* Major version bump 3.0.0

* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* Tdl 23543 fix uuid transformation (#100)

* - set maxDiff to None to reset dict comparison limit
- sort the dictionary before assertion

* fix uuid transformation

* upgrade pymongo version in cci configuration

* fix cci configuration

* upgrade pip and setuptools

* update cci config to run on latest python version

* fix linting issues

* fix uuid bookmarking

* fix integration tests

* remove debugger import

* fix integration test compatibility issues with pymongo v4.3+ and mongoddb v6.0

* - fix transaction life time limit
- improve integration testlogging

* fix typo

* remove f-string from logger statements

* minor code cleanup

* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* - set maxDiff to None to reset dict comparison limit
- sort the dictionary before assertion

* fix uuid transformation

* upgrade pymongo version in cci configuration

* fix cci configuration

* upgrade pip and setuptools

* update cci config to run on latest python version

* fix linting issues

* fix uuid bookmarking

* fix integration tests

* remove debugger import

* fix integration test compatibility issues with pymongo v4.3+ and mongoddb v6.0

* - fix transaction life time limit
- improve integration testlogging

* fix typo

* remove f-string from logger statements

* minor code cleanup

* [Tdl 23471] Fix circle ci config (#101)

* Added CircleCI config for mongodb 6.0.

* Added Ciall testsoin CCI for ngodb 6.5.0 & 6.0

* Fixed CCI config and added all tests for mongodb 4.4

* Removed mongodb 4.2 testing from CCi as it has reached end of life.

* Fixed redundant statement to include all integration tests.

* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* - set maxDiff to None to reset dict comparison limit
- sort the dictionary before assertion

* fix uuid transformation

* upgrade pymongo version in cci configuration

* fix cci configuration

* upgrade pip and setuptools

* update cci config to run on latest python version

* fix linting issues

* fix uuid bookmarking

* fix integration tests

* remove debugger import

* fix integration test compatibility issues with pymongo v4.3+ and mongoddb v6.0

* - fix transaction life time limit
- improve integration testlogging

* Added CircleCI config for mongodb 6.0.

* Added Ciall testsoin CCI for ngodb 6.5.0 & 6.0

* Fixed CCI config and added all tests for mongodb 4.4

* Removed mongodb 4.2 testing from CCi as it has reached end of life.

* Fixed redundant statement to include all integration tests.

* Removed glob & for loop statements as all integration tests are running in CCi.

---------

Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.com>

* Tdl 23609 fix empty projection (#102)

* projection test change

* - fix the emtpy projection issue
- fix the integration tests

* minor integration test fixes

* remove redundant comments

* Force {} projection to be None

* Changes:
1) Upgrade pymongo to 4.4.0
2) Added condition to handle DatetimeMS binary date values.

* - set maxDiff to None to reset dict comparison limit
- sort the dictionary before assertion

* fix uuid transformation

* upgrade pymongo version in cci configuration

* fix cci configuration

* upgrade pip and setuptools

* update cci config to run on latest python version

* fix linting issues

* fix uuid bookmarking

* fix integration tests

* remove debugger import

* fix integration test compatibility issues with pymongo v4.3+ and mongoddb v6.0

* - fix transaction life time limit
- improve integration testlogging

* projection test change

* - fix the emtpy projection issue
- fix the integration tests

* minor integration test fixes

* remove redundant comments

* fix typo

* remove f-string from logger statements

* Force {} projection to be None

---------

Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.com>
Co-authored-by: shantanu73 <shantanudhiman25@gmail.com>

---------

Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.com>
Co-authored-by: shantanu73 <shantanudhiman25@gmail.com>

* Removed CCi changes from CHANGELOG.md

* renamed build_mongo_4_4 to build in config.yml

* Renamed build_mongo_4_4 to build in config.yml

* Major version bump 3.0.0

* Removed CCi changes from CHANGELOG.md

---------

Co-authored-by: Rushikesh Todkar <98420315+RushiT0122@users.noreply.github.com>
Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.com>
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

4 participants