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

Baseline code to query Spanner using DynamoDB queries #3

Merged
merged 60 commits into from Mar 17, 2021

Conversation

rosspatil
Copy link
Collaborator

Fixes #2

  • Tests pass
  • Appropriate changes to README are included in PR

@rosspatil rosspatil added the good first issue Good for newcomers label Jul 12, 2020
@googlebot googlebot added the cla: yes CLA signed label Jul 12, 2020
@lgruen
Copy link

lgruen commented Jul 14, 2020

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.

@googlebot
Copy link

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.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no CLA not signed and removed cla: yes CLA signed labels Jul 20, 2020
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes CLA signed and removed cla: no CLA not signed labels Jul 20, 2020
@agasheesh agasheesh self-requested a review July 25, 2020 06:38
@rosspatil rosspatil changed the base branch from master to baseline August 17, 2020 04:39
@google-cla google-cla bot added cla: yes CLA signed and removed cla: no CLA not signed labels Jan 20, 2021
Copy link
Collaborator

@bgood bgood left a 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.

integrationtest/setup.go Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@google-cla
Copy link

google-cla bot commented Jan 20, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Jan 20, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Jan 20, 2021

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.
@google-cla
Copy link

google-cla bot commented Jan 20, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@cloudspannerecosystem cloudspannerecosystem deleted a comment from google-cla bot Jan 20, 2021
@cloudspannerecosystem cloudspannerecosystem deleted a comment from google-cla bot Jan 20, 2021
@skuruppu skuruppu dismissed hengfengli’s stale review January 20, 2021 23:13

@hengfengli is OOO and we will do another review of this when merging from baseline to master.

@google-cla
Copy link

google-cla bot commented Feb 1, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@bgood
Copy link
Collaborator

bgood commented Mar 17, 2021

Based on conversation with @skuruppu I captured most of the open comments as Issues #14 - #23 and will merge into the baseline branch.

@bgood bgood merged commit 3c5c199 into cloudspannerecosystem:baseline Mar 17, 2021
bgood added a commit that referenced this pull request Nov 11, 2021
* 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 <koloite@gmail.com>

* 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 <ankit.malik@cldcvr.com>
Co-authored-by: Knut Olav Løite <koloite@gmail.com>
Co-authored-by: Saurav Ghosh <saurav@cldcvr.com>

* Updates for automated testing (#25)

Co-authored-by: skuruppu <skuruppu@google.com>

* 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 <skuruppu@google.com>

Co-authored-by: skuruppu <skuruppu@google.com>

* 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 <patilroshan443@gmail.com>
Co-authored-by: ankitmalikg2 <ankit.malik@cldcvr.com>
Co-authored-by: Knut Olav Løite <koloite@gmail.com>
Co-authored-by: Saurav Ghosh <saurav@cldcvr.com>
Co-authored-by: skuruppu <skuruppu@google.com>
Co-authored-by: Shobhit Gupta <43795024+gushob21@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA signed good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Baseline code to query Cloud Spanner using DynamoDB queries.
9 participants