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-19742 Run discover mode if catalog is not given in sync mode #85

Conversation

jtilala
Copy link
Contributor

@jtilala jtilala commented Jul 1, 2022

Description of change

  • Run discover mode and generate catalog when the catalog.json is not given in sync mode.
  • Write a unit test to justify the above scenario.

Manual QA steps

Risks

Rollback steps

  • revert this branch

@@ -0,0 +1,38 @@
from shutil import ExecError
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this unused import

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 unused imported packages.

Comment on lines 23 to 24
mocked_parse_args.return_value = MockedParseArgs
mocked_google_client.return_value = "test"
Copy link
Contributor

Choose a reason for hiding this comment

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

Move these two lines at the @mock.patch decorator level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added return_value at decorator level and removed it from the test function

mocked_parse_args.return_value = MockedParseArgs
mocked_google_client.return_value = "test"
main()
mocked_sync.assert_called_with(client="test", config=MockedParseArgs.config, catalog="test", state={})
Copy link
Contributor

@savan-chovatiya savan-chovatiya Jul 1, 2022

Choose a reason for hiding this comment

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

Add additional assertion like discover() should be called for 0 or 1 times.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added assertion to check whether or not sync function calls discover()

@hpatel41 hpatel41 requested a review from luandy64 July 19, 2022 10:49
@hpatel41 hpatel41 changed the base branch from master to crest-master August 10, 2022 05:33
@hpatel41 hpatel41 merged commit 2b3ba82 into crest-master Aug 10, 2022
@hpatel41 hpatel41 mentioned this pull request Aug 10, 2022
RushiT0122 pushed a commit that referenced this pull request Sep 1, 2022
* TDL-19742 Run discover mode if catalog is not given in sync mode (#85)

* TDL-19742 Run discover mode if catalog is not given in sync mode

* Add call count assertion and remove unused import

Co-authored-by: Jay Tilala <jay.tilala@CDSYS.LOCAL>

* TDL-14450: fix inconsistency of empty cell for boolean (#84)

* handled empty value scenario for the booleans

* Added unit test for null value in boolean

* Removed workaround from the integration test

Co-authored-by: harshpatel4crest <harsh.patel4@crestdatasys.com>

* TDL-14449: Syncs do not parse datetime values in number datatyped columns as strings (#83)

* updated the code to store datetime values as string in numberType column

* updated the test cases and handled boolean in numberType

* updated code for handling numberType values

* updated code to handle number format and int exponential values

* added comments for transforming number format data

* updated datatype integration test

* resolved comments

* resolved comments

* updated comments

* updated comments

* handled duplicate code in transform numberType values

* added comments

* added logger and removed redundant arg

* updated unittest and code

* updated comment

* updated bug id test case

* TDL-19029: Add support for data collection from the shared drives (#80)

* TDL-17517 - Add missing tap-tester cases (#65)

* Added missing test cases

* Updated standard bookmark test name

* Added back bookmark for full_table stream

* Updated comment

* Removed bookmark test case.

* TDL-17698: Dict based to class based refactoring (#66)

* refactored code to class based

* resolve unittest failure

* updated the code to write state after syncing records for file metadata stream

* added code change to return if file is not changed

* added code change to write file metadata bookmark at the end of the sync

* added function comments

* created a function to get path with query params

* updated code according to pylint

* resolved unittest failure

* TDL-19029: Add support for data collection from the shared drives

* change streams.py

* update README for supportsAllDrives

* remove invalid datatype exception

* include false as string or bool

* add unitest

* updated error message

* change default value

* update comments

* update parameter name snake case

* solved circleci error

* update comments in unittest

* Add exception for the invalid value of the supports_all_drives

* optimize code as per collin suggestion

* Addressed andy's comments

* rename import sync name

* rename import sync name

* updated unittest

Co-authored-by: Prijen Khokhani <88327452+prijendev@users.noreply.github.com>
Co-authored-by: Harsh <80324346+harshpatel4crest@users.noreply.github.com>
Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com>
Co-authored-by: Jay Tilala <jay.tilala@CDSYS.LOCAL>
Co-authored-by: harshpatel4crest <harsh.patel4@crestdatasys.com>

* added param for shared drive for all syncs (#89)

Co-authored-by: jtilala <104966482+jtilala@users.noreply.github.com>
Co-authored-by: Jay Tilala <jay.tilala@CDSYS.LOCAL>
Co-authored-by: savan-chovatiya <80703490+savan-chovatiya@users.noreply.github.com>
Co-authored-by: Prijen Khokhani <88327452+prijendev@users.noreply.github.com>
Co-authored-by: prijendev <prijen.khokhani@crestdatasys.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.

6 participants