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.

Sign up for GitHub

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

Jump to bottom

[AD-517] Adapt dsn config #13

Merged
merged 22 commits into from
Feb 3, 2022
Merged

[AD-517] Adapt dsn config #13

merged 22 commits into from
Feb 3, 2022

Conversation

andiemontoyeah
Copy link
Contributor

@andiemontoyeah andiemontoyeah commented Jan 20, 2022 •
edited
Loading

Summary

Removed ignite connection properties and replaced with DocumentDB properties

Description

  • Replaced properties in configuration.h and connection_string_parser.cpp with DocumentDB connection properties
  • Added 4 files - enums for scan method and read preference properties
  • Re-enabled tests in configuration_test - mostly around connection string parsing
  • Connection string validation is incomplete - need to check that required properties are passed and throw error on invalid properties or combinations of properties - extracted to a separate task
  • DSN configuration window is still showing the old Ignite properties - this was extracted to a separate task

Related Issue

https://bitquill.atlassian.net/browse/AD-517

Additional Reviewers

@affonsoBQ
@alexey-temnikov
@andiem-bq
@birschick-bq
@garya-bitquill

birschick-bq
birschick-bq reviewed Jan 21, 2022
View reviewed changes
src/odbc/src/config/configuration.cpp Outdated Show resolved Hide resolved
src/odbc/src/config/connection_string_parser.cpp Outdated Show resolved Hide resolved
src/odbc/src/message.cpp Outdated Show resolved Hide resolved
src/odbc/include/ignite/odbc/config/connection_string_parser.h Outdated Show resolved Hide resolved
@garya-bitquill garya-bitquill mentioned this pull request Jan 21, 2022
Closed
@andiemontoyeah andiemontoyeah marked this pull request as ready for review January 21, 2022 02:19
birschick-bq
birschick-bq approved these changes Jan 27, 2022
View reviewed changes
Copy link
Contributor

@birschick-bq birschick-bq left a comment

Choose a reason for hiding this comment

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

One small change. Otherwise - good to go!

src/odbc/src/connection.cpp Outdated Show resolved Hide resolved
@andiemontoyeah andiemontoyeah merged commit c728b69 into develop Feb 3, 2022
@andiemontoyeah andiemontoyeah deleted the andiem/AD-517/adapt_dsn_config branch February 3, 2022 20:57
affonsov pushed a commit that referenced this pull request Nov 16, 2022
@jerrytfleung
Merge pull request #13 from Bit-Quill/AT-637
6315d87 
At 637
affonsov added a commit that referenced this pull request Nov 16, 2022
@affonsov
Create CONTRIBUTING.md (#13)
1f204c5
affonsov pushed a commit that referenced this pull request Nov 16, 2022
@andiemontoyeah
[AD-517] Adapt dsn config (#13)
fd1ffa8 
* [AD-517] Remove old properties + add new properties

* [AD-517] Re-enable unit tests - WIP -  still need to cleanup

* Add more unit tests

* Remove extra file

* Remove redundant declaration

* [AD-517] * Change the C++ version to use to compile.

* [AD-517] * Changed checks.yml so that it doesn't fail on code review errors.

* Improvements from review

* Updated comments

* Removed includes no longer being used in configuration.h

* Some formatting improvements

* Improvements from code review + address some warnings

* Change connection _test to use hostname key instead of address

* Change connection_test to use databse key instead of schema key

* Attempt to fix windows 32 build

* Fix formatting

* Improvements from code review

* Improvement from code review - Remove new line between enum types

* Change cmake version to latest for windows

* Update cmake version to latest

Co-authored-by: Bruce Irschick <[email protected]>
affonsov pushed a commit that referenced this pull request Nov 17, 2022
@jerrytfleung
Merge pull request #13 from Bit-Quill/AT-637
dc3e3ce 
At 637
affonsov added a commit that referenced this pull request Nov 17, 2022
@affonsov
Create CONTRIBUTING.md (#13)
ba3c46e
affonsov pushed a commit that referenced this pull request Nov 17, 2022
@andiemontoyeah
[AD-517] Adapt dsn config (#13)
9ba1790 
* [AD-517] Remove old properties + add new properties

* [AD-517] Re-enable unit tests - WIP -  still need to cleanup

* Add more unit tests

* Remove extra file

* Remove redundant declaration

* [AD-517] * Change the C++ version to use to compile.

* [AD-517] * Changed checks.yml so that it doesn't fail on code review errors.

* Improvements from review

* Updated comments

* Removed includes no longer being used in configuration.h

* Some formatting improvements

* Improvements from code review + address some warnings

* Change connection _test to use hostname key instead of address

* Change connection_test to use databse key instead of schema key

* Attempt to fix windows 32 build

* Fix formatting

* Improvements from code review

* Improvement from code review - Remove new line between enum types

* Change cmake version to latest for windows

* Update cmake version to latest

Co-authored-by: Bruce Irschick <[email protected]>
alexey-temnikov pushed a commit that referenced this pull request Jan 14, 2023
@jerrytfleung
Merge pull request #13 from Bit-Quill/AT-637
4345300 
At 637
alexey-temnikov pushed a commit that referenced this pull request Jan 14, 2023
@affonsov
Create CONTRIBUTING.md (#13)
becbb54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@affonsov affonsov affonsov approved these changes

@birschick-bq birschick-bq birschick-bq approved these changes

@alexey-temnikov alexey-temnikov Awaiting requested review from alexey-temnikov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andiemontoyeah @alexey-temnikov @birschick-bq @affonsov