-
Notifications
You must be signed in to change notification settings - Fork 11
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
Baseline code to query Spanner using DynamoDB queries #3
Conversation
Thanks a lot for sending this PR, @rosspatil! Any chance we could split it up a bit, to make it easier to review? At the moment it's almost 8k lines of code. This way, we could also do some of the reviews in parallel. |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
DYN-40: integration testing framework added
DYN-39: added base functions for dynamodb similar output
Dyn 41 : GetItem API implementation and test cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update, missing header and a couple nits in the README.
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
- Fixed environment name. - Replaced `.` with `-` for the config file names to match the expected conterpart. - Added copyright header to integrationtest/setup.go.
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
@hengfengli is OOO and we will do another review of this when merging from baseline to master.
…on tests. Also, added CONTRIBUTING.md file
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
* Baseline code to query Spanner using DynamoDB queries (#3) * Baseline code to query Spanner using DynamoDB queries * readme file * DYN-40: integration testing framework added * DYN-39: added base functions for output change * DYN-41: GetItem API added * DYN-41 test cases imporved * DYN-41: getItem API output & test case changed * DYN-43: BatchGet api input and output changes done * DYN-43: test cases added & end-point correction * DYN-41: Query api change * DYN-42: test cases added * DYN-42: query api output & testcases changed * DYN-44: scan input & output changed * DYN-44: corrected url & added test cases * DYN-31: input & output chnages for UpdateItem * DYN-31: Put API input & output changes * DYN-31: test cases & output change for Update * DYN-31: removed commented code for test * DYN-47: DeleteItem API implementation & test cases * DYN-48: BatchWriteItem API input changes * DYN-48: BatchWriteItem API test cases added * DYN-48: test cases for spanner has been corrected * DYN-47: Delete API test cases * corrected 1 test case * DYN-33: initial test data has been added * DYN-51: imporved readme file * utils: unit test added * config_test added * condition test cases added * services test cases added * test case for services added * CCI03-52 : license header added * CCI03-52: package comments added * Update README.md Co-authored-by: Knut Olav Løite <[email protected]> * CCI03-53: readme file & config file improved * prod config file improved * added proper space in readme * fix : issues related linting fixed * fix: linting issues for log and model fixed * fix: logger_test linting issue fixed * fix: logger_test linting issue fixed * updated README.md and configured integration test to run setup and cleanup logic * updated config to contain description * Fix: Typos - Fixed environment name. - Replaced `.` with `-` for the config file names to match the expected conterpart. - Added copyright header to integrationtest/setup.go. * integration test: to check ddl table updated correctly before execution tests. Also, added CONTRIBUTING.md file Co-authored-by: ankitmalikg2 <[email protected]> Co-authored-by: Knut Olav Løite <[email protected]> Co-authored-by: Saurav Ghosh <[email protected]> * Updates for automated testing (#25) Co-authored-by: skuruppu <[email protected]> * Adding a starter .gitignore to the project. (#26) * Changing up the config loader and documentation to simplify the code and support more than just production and staging environments. (#31) * Simplifying the logger initilization and wrappers. (#32) * Adding router function to parse X-Amz-Target and send the request to the appropiate handler. (#34) * Fixing queryResponse to grab the correct results list. (#36) * Adding support for Numeric data type. (#35) * Example application written in Golang (#27) * Initial commit of example data, scripts and golang application. * Added more read access patterns * Pulling region from env vars. * Updates to handle the query endpoints for the adatper. * Adding config for the indexes. * Removing the code to make extra sessions. * Updating to go 1.17 * Cleaning up the README some and updating to the new config file names. * Found a couple more places were config file names needed to be fixed. * Apply suggestions from code review Co-authored-by: skuruppu <[email protected]> Co-authored-by: skuruppu <[email protected]> * Various fixes for linting and tests (#33) * Fixing broken test in config_test.go * Fixing linter errors. * Cleaning up redundant declaration and adding skip for short tests. * Updates to the CircleCI config to increase parallerism and fix linting. * Syntax fix in the circleci config. * Cleaning up deadcode linting errors. * Cleaning up missing error checks. * Cleaning up some unused code. * Fixing gosimple errors from the linter. * Fixing a missing err check. * more gosimple fixes. * More linter fixes. * increasing the linter timeout. * Cleaning up the last few linter errors. * Fixed another staticcheck error. * Output updates for the integraiton tests. * Moved changeTableNameForSP to utils and updated other files use the new centralized version. (#40) * Documentation updates and clean up (#41) * Add compatibility info and resturctured READMEs. Restructured the README to reference the examples and moved the integration testing instructions to their own README. Also added supported DynamoDB operations and data types to the README. * Minor tweaks and spelling fixes. * Fixes for issues found during review. * CircleCI config and sytles for markdown linting. * Linting fixes. * Changing the matching to use regex and added tests for varying cases. (#49) * Changing the matching to use regex and added tests for varying cases. * Changing the regexes to use the case insensitive flag. * Adding models for the BatchWriteItem response and error handling. (#50) * Adding example functions for more DB operations (#48) * Adding UpdateItem example * Adding UpdateItem example * Adding PutItem example * Adding DeleteItem example * Adding ScanItem example * Adding more example functions * Adding BatchWriteItem example * Adding BatchWriteItem example * Fixing indentation and moving some code * Fixing indentation and moving some code * Fixing indentation and moving some code * Fixing indentation and moving some code * Fixing indentation and moving some code * Fixing indentation and moving some code * Fixing indentation and moving some code * Starting to add tests and some refactor for storage/spanner.go (#44) Changes: - Added spanner_test.go. Tests currently on cover parseRow - Renamed parseRowForNull to parseRow (issue #22) - createRowMap and parseRow did pretty much the same thing so deleted createRowMap and updated reference to use parseRow - Removed the unneeded cols argument from parseRow * Fixes for integration test failures in CircleCI (#54) * Changing to use the working directory for the config files and directly export the credentials environment var. * Removing branch restriction from integration_test * Fixing the spanner.json config to use the var Co-authored-by: Roshan Patil <[email protected]> Co-authored-by: ankitmalikg2 <[email protected]> Co-authored-by: Knut Olav Løite <[email protected]> Co-authored-by: Saurav Ghosh <[email protected]> Co-authored-by: skuruppu <[email protected]> Co-authored-by: Shobhit Gupta <[email protected]>
Fixes #2