Feature/oracle support #702

Curlack · 2023-10-19T21:38:38Z

This covers all but one task. Not sure what to do for "Create a derived OracleManagedTestProvider class to derive from...".
These changes are for baseline Oracle setup. To confirm setup and build scripts work I ran PetaPoco.Tests.Integration.DatabaseTests.Construct_GivenConnection_ShouldBeValid, which executed successfully.

Add Oracle image to docker-compose.yml. Update connection strings in app.config and appsetting.json. Include "Oracle.ManagedDataAccess" nuget package. Create Oracle -Setup and -Build scripts. Create base OracleTestProvider. Add OracleDatabaseTests. Fix TODO I came accross in ParameterHelper class. Fix script parsing issue found in code borrowed from MSAccessTestProvider during Oracle ExecuteBuildScript. Introduce new ExecutionPhase enum in TestProvider to accomodate setup and build phases necessary in OracleTestProvider. NOTE: Added SuppressTfmSupportBuildWarnings in project file to circumvent "System.Runtime.CompilerServices.Unsafe doesn't support netcoreapp2.1" build error. Not sure how else to fix this issue.

Ste1io · 2023-10-20T03:23:09Z

Looks fantastic based on a quick fly-by over the commit @Curlack. I'll take a look at everything between now and tomorrow.

PetaPoco/Utilities/ParametersHelper.cs

Ste1io

Fix script parsing issue found in code borrowed from MSAccessTestProvider during Oracle ExecuteBuildScript.

I'm curious what the script parsing bug was? Any chance it was related to 7e6c940?

Curlack · 2023-10-20T05:27:27Z

When I reused that part of the code, to also cater for comments in the script, I didn't realize that the semi colon was required. During troubleshooting, I found that the next line was jammed to the back of the comment seperated by '\r\n'. Then I realized the code should rather check lines within semi-colon seperated block of text. Since that was gonna fix my issue, I also improved the part of the code I borrowed it from. Don't think it is related though.

…

On Fri, 20 Oct 2023, 7:16 am Stelio Kontos ***@***.***> wrote: ***@***.**** commented on this pull request. I'm curious what the script parsing bug was? Any chance it was related to 7e6c940 <7e6c940> ? — Reply to this email directly, view it on GitHub <#702 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACI4HPBOWIQNA6PBVSP3FTYAICMPAVCNFSM6AAAAAA6H34TQSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMOBZGI2DMMBYGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

PetaPoco.Tests.Integration/Providers/OracleTestProvider.cs

Ste1io · 2023-10-20T06:36:10Z

Everything looks good @Curlack. Just one non-code-related typo, which isn't me attempting to be nitpicky I promise. About the OracleManagedTestProvider, that really isn't necessary until/if another provider is added. That was mainly in light of the DbProviderFactory for Oracle.

Co-authored-by: Stelio Kontos <37424493+Ste1io@users.noreply.github.com>

Curlack · 2023-10-20T06:43:20Z

Excellent, thanks. Committed your suggestion. Not used to aal these github features lol. Will learn I suppose.

…

On Fri, 20 Oct 2023, 8:36 am Stelio Kontos ***@***.***> wrote: Everything looks good @Curlack <https://github.com/Curlack>. Just one non-code-related typo, which isn't me attempting to be nitpicky I promise. About the OracleManagedTestProvider, that really isn't necessary until/if another provider is added. That was mainly in light of the DbProviderFactory for Oracle. — Reply to this email directly, view it on GitHub <#702 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACI4HLVWJZJWKAVH5NAKMDYAILWLAVCNFSM6AAAAAA6H34TQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZSGE3DOMJXHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Ste1io · 2023-10-20T06:53:15Z

Not used to aal these github features lol. Will learn I suppose.

I can relate...always afraid it will make some assumption I don't want and I'll regret it later (probably a result of back when I started using rebase for private branches and realized "Pull" defaults to Fetch/Merge), so tend to using VSCode integration or CLI directly in most cases.

Ste1io · 2023-10-20T07:12:42Z

NOTE: Added SuppressTfmSupportBuildWarnings in project file to circumvent "System.Runtime.CompilerServices.Unsafe doesn't support netcoreapp2.1" build error. Not sure how else to fix this issue.

I checked out your branch and looking into this...with the warning suppressed, it builds but the test fails when ran under dotnet2.1 target. The offending package causing that error message is Oracle.ManagedDataAccess; which is odd that it doesn't affect net472, seeing as that dependency is considered the most current iirc.

For reference:

2>------ Rebuild All started: Project: PetaPoco.Tests.Integration, Configuration: Debug Any CPU ------
2>C:\Users\there\.nuget\packages\system.runtime.compilerservices.unsafe\6.0.0\buildTransitive\netcoreapp2.0\System.Runtime.CompilerServices.Unsafe.targets(4,5): error : System.Runtime.CompilerServices.Unsafe doesn't support netcoreapp2.1. Consider updating your TargetFramework to netcoreapp3.1 or later.
2>Done building project "PetaPoco.Tests.Integration.csproj" -- FAILED.

Curlack · 2023-10-20T07:23:13Z

FYI, I wasn't able to build even before adding the Oracle nuget package. Would like to know or try to figure out what to do so my build doesn't fail without that change. For the time being used it like that to concentrate on the important parts (I get distracted very easy lol). Thought of not including it in the PR, but figured you'd guide in the right direction if I do include it, 'no harm no foul' kind of thing.

…

On Fri, 20 Oct 2023, 9:12 am Stelio Kontos ***@***.***> wrote: NOTE: Added SuppressTfmSupportBuildWarnings in project file to circumvent "System.Runtime.CompilerServices.Unsafe doesn't support netcoreapp2.1" build error. Not sure how else to fix this issue. I checked out your branch and looking into this...with the warning suppressed, it builds but the test fails when ran under dotnet2.1 target. The offending package causing that error message is Oracle.ManagedDataAccess; which is odd that it doesn't affect net472, seeing as that dependency is considered the most current iirc. For reference: 2>------ Rebuild All started: Project: PetaPoco.Tests.Integration, Configuration: Debug Any CPU ------ 2>C:\Users\there\.nuget\packages\system.runtime.compilerservices.unsafe\6.0.0\buildTransitive\netcoreapp2.0\System.Runtime.CompilerServices.Unsafe.targets(4,5): error : System.Runtime.CompilerServices.Unsafe doesn't support netcoreapp2.1. Consider updating your TargetFramework to netcoreapp3.1 or later. 2>Done building project "PetaPoco.Tests.Integration.csproj" -- FAILED. — Reply to this email directly, view it on GitHub <#702 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACI4HON452C33MTPEGHK5TYAIP7LAVCNFSM6AAAAAA6H34TQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZSGIYDKMZUGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Ste1io · 2023-10-20T07:47:55Z

FYI, I wasn't able to build even before adding the Oracle nuget package.
Would like to know or try to figure out what to do so my build doesn't fail
without that change.

Can you copy the output log in here for me with the suppression and Oracle.ManagedDataAccess package removed from the project file?

For the time being used it like that to concentrate on
the important parts

Completely understand, I'm the same. Hence the TODO you found ;)

but figured you'd guide in the right direction if I
do include it, 'no harm no foul' kind of thing.

No harm done.

Ste1io · 2023-10-20T07:55:19Z

The offending package causing that error message is Oracle.ManagedDataAccess; which is odd that it doesn't affect net472, seeing as that dependency is considered the most current iirc.

OK, I was wrong here; Oracle.ManagedDataAccess is the framework version (hence no issues with net472 target). Oracle.ManagedDataAccess.Core is the "modern" package for core apps (click links for compatible versions on nuget).

Unfortunately, the lowest compatable dotnetcore version for the "Oracle.ManagedDataAccess.Core" package is dotnetcore3.1. Any ideas @Curlack?

Ste1io · 2023-10-20T08:08:29Z

Screw it, tests are getting updated to netcoreapp3.1. I'm running the full test suite rn, and will push the targetframeworks change in a separate PR to this branch to isolate it from these changes.

Curlack · 2023-10-20T08:21:10Z

Sorry dude, not in front of my laptop atm. Had a good laugh after reading you comments. Does that mean you don't need the output logs from me anymore? Cause I'll only be able to give them to you this evening when I get home.

…

On Fri, 20 Oct 2023, 10:08 am Stelio Kontos ***@***.***> wrote: Screw it, tests are getting updated to netcoreapp3.1. I'm running the full test suite rn, and will push the targetframeworks change in a separate PR to this branch to isolate it from these changes. [image: 1697789162-PetaPoco_-_Microsoft_Visual_Studio] <https://user-images.githubusercontent.com/37424493/276864787-de7cb9d0-e888-4316-9d64-be1b8c9f72bb.png> — Reply to this email directly, view it on GitHub <#702 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACI4HMLKAYBZ54MCYPK7ELYAIWQRAVCNFSM6AAAAAA6H34TQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZSGI3TMMRVHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Ste1io

Let's remove the suppression, and add a condition for the package. Oracle tests won't pass for netcoreapp2.1, but it should build fine (ie, no change from current state). Sets it up so we can roll in the 2.1->3.1 upgrade smoothly though.

PetaPoco.Tests.Integration/PetaPoco.Tests.Integration.csproj

Co-authored-by: Stelio Kontos <37424493+Ste1io@users.noreply.github.com>

Ste1io · 2023-10-20T08:54:44Z

Had a good laugh after reading you comments.

Inline code comments or my half hazard PR thread comments? haha

Does that mean you don't need the output logs from me anymore?

If it compiled fine prior to adding the Oracle dependency, it's most likely because that package is not compatable with dotnetcore2.1. Removing the suppression broke my build also. If that's the case, no need sending the logs as updating the target should fix it for you as it did for me. If, after rebasing your branch onto my upcoming commit with the upgrade, you still have issues, let me know, but it should resolve it.

…ure/oracle-support

Ste1io

When you're at your pc and sync'd up with the changes here locally, @Curlack, could you rebase your feature branch onto upstream/feature/oracle-support, then add Oracle.ManagedDataAccess.Core package conditional to netcoreapp3.1 to the integration tests project file? Once that's committed/pushed this pr will be all set.

PetaPoco.Tests.Integration/PetaPoco.Tests.Integration.csproj

Curlack · 2023-10-20T21:29:28Z

When I looked at the file I thought the same thing, but I didn't put it there. After installing the nuget package, it was like that all by itself lol.

…

On Fri, 20 Oct 2023, 11:09 pm Stelio Kontos ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In PetaPoco.Tests.Integration/PetaPoco.Tests.Integration.csproj <#702 (comment)> : > + <PackageReference Include="Oracle.ManagedDataAccess.Core"> + <Version>3.21.120</Version> + </PackageReference> Given the current structure of this project file, this probably should be dropped in with the other package references IMO. It seems there's no right way of laying out these files, but grouping this with the list removal items is a sure-fire way of confusing yours truly some late night eventually. 😆 — Reply to this email directly, view it on GitHub <#702 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACI4HMK7HK5YEKLLS74ALLYALR7DAVCNFSM6AAAAAA6H34TQSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMOJQHEYDIOJYGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Curlack · 2023-10-20T21:29:53Z

Thanks

…

On Fri, 20 Oct 2023, 11:09 pm Stelio Kontos ***@***.***> wrote: ***@***.**** approved this pull request. — Reply to this email directly, view it on GitHub <#702 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACI4HI6SJKZTCRJCD5WIYDYALSAZAVCNFSM6AAAAAA6H34TQSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMOJQHEYDKMZYG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Ste1io · 2023-10-20T21:41:53Z

When I looked at the file I thought the same thing, but I didn't put it there. After installing the nuget package, it was like that all by itself lol.

No worries. I'm set to merge soon as that's sorted. Any issues building at this point?

Curlack · 2023-10-20T21:43:29Z

No issues. Smooth sailing all the way. I had to install the 3.1 sdk and runtime though.

…

On Fri, 20 Oct 2023, 11:42 pm Stelio Kontos ***@***.***> wrote: When I looked at the file I thought the same thing, but I didn't put it there. After installing the nuget package, it was like that all by itself lol. No worries. I'm set to merge soon as that's sorted. Any issues building at this point? — Reply to this email directly, view it on GitHub <#702 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACI4HIDVVFVLGDJ2XNZLGDYALV2ZAVCNFSM6AAAAAA6H34TQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZTGQZDQOJZG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Ste1io · 2023-10-20T22:47:29Z

Looks fantastic @Curlack. Let's get this Oracle merged in and see how he fairs with the tests :)

Curlack added 2 commits October 19, 2023 23:25

Remove unused code.

94f4e7c

Ste1io reviewed Oct 20, 2023

View reviewed changes

PetaPoco/Utilities/ParametersHelper.cs Show resolved Hide resolved

Ste1io reviewed Oct 20, 2023

View reviewed changes

PetaPoco.Tests.Integration/Providers/OracleTestProvider.cs Outdated Show resolved Hide resolved

Ste1io mentioned this pull request Oct 20, 2023

Add support for Oracle: implementation and tests #699

Open

9 tasks

Update PetaPoco.Tests.Integration/Providers/OracleTestProvider.cs

9ce545a

Co-authored-by: Stelio Kontos <37424493+Ste1io@users.noreply.github.com>

Curlack closed this Oct 20, 2023

Curlack reopened this Oct 20, 2023

Ste1io requested changes Oct 20, 2023

View reviewed changes

PetaPoco.Tests.Integration/PetaPoco.Tests.Integration.csproj Outdated Show resolved Hide resolved

PetaPoco.Tests.Integration/PetaPoco.Tests.Integration.csproj Outdated Show resolved Hide resolved

Curlack and others added 2 commits October 20, 2023 10:29

Update PetaPoco.Tests.Integration/PetaPoco.Tests.Integration.csproj

f306d02

Co-authored-by: Stelio Kontos <37424493+Ste1io@users.noreply.github.com>

Update PetaPoco.Tests.Integration/PetaPoco.Tests.Integration.csproj

bd743af

Co-authored-by: Stelio Kontos <37424493+Ste1io@users.noreply.github.com>

Ste1io approved these changes Oct 20, 2023

View reviewed changes

Ste1io mentioned this pull request Oct 20, 2023

Upgrade tests projects target framework to net472,netcoreapp3.1 #703

Closed

Merge branch 'CollaboratingPlatypus:feature/oracle-support' into feat…

d731374

…ure/oracle-support

Ste1io requested changes Oct 20, 2023

View reviewed changes

Add Oracle.ManagedDataAccess.Core for netcoreapp3.1 framework.

682d6b3

Curlack requested a review from Ste1io October 20, 2023 20:52

Ste1io reviewed Oct 20, 2023

View reviewed changes

PetaPoco.Tests.Integration/PetaPoco.Tests.Integration.csproj Outdated Show resolved Hide resolved

Ste1io approved these changes Oct 20, 2023

View reviewed changes

Put Package References together

9b5b929

Ste1io merged commit eaf8e5f into CollaboratingPlatypus:feature/oracle-support Oct 20, 2023

Ste1io mentioned this pull request Nov 5, 2023

Fix PocoData Cache Issue #711

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/oracle support #702

Feature/oracle support #702

Curlack commented Oct 19, 2023

Ste1io commented Oct 20, 2023

Ste1io left a comment •

edited

Curlack commented Oct 20, 2023 via email

Ste1io commented Oct 20, 2023

Curlack commented Oct 20, 2023 via email

Ste1io commented Oct 20, 2023 •

edited

Ste1io commented Oct 20, 2023

Curlack commented Oct 20, 2023 via email

Ste1io commented Oct 20, 2023

Ste1io commented Oct 20, 2023 •

edited

Ste1io commented Oct 20, 2023

Curlack commented Oct 20, 2023 via email

Ste1io left a comment

Ste1io commented Oct 20, 2023 •

edited

Ste1io left a comment

Curlack commented Oct 20, 2023 via email

Curlack commented Oct 20, 2023 via email

Ste1io commented Oct 20, 2023

Curlack commented Oct 20, 2023 via email

Ste1io commented Oct 20, 2023

Feature/oracle support #702

Feature/oracle support #702

Conversation

Curlack commented Oct 19, 2023

Ste1io commented Oct 20, 2023

Ste1io left a comment • edited

Choose a reason for hiding this comment

Curlack commented Oct 20, 2023 via email

Ste1io commented Oct 20, 2023

Curlack commented Oct 20, 2023 via email

Ste1io commented Oct 20, 2023 • edited

Ste1io commented Oct 20, 2023

Curlack commented Oct 20, 2023 via email

Ste1io commented Oct 20, 2023

Ste1io commented Oct 20, 2023 • edited

Ste1io commented Oct 20, 2023

Curlack commented Oct 20, 2023 via email

Ste1io left a comment

Choose a reason for hiding this comment

Ste1io commented Oct 20, 2023 • edited

Ste1io left a comment

Choose a reason for hiding this comment

Curlack commented Oct 20, 2023 via email

Curlack commented Oct 20, 2023 via email

Ste1io commented Oct 20, 2023

Curlack commented Oct 20, 2023 via email

Ste1io commented Oct 20, 2023

Ste1io left a comment •

edited

Ste1io commented Oct 20, 2023 •

edited

Ste1io commented Oct 20, 2023 •

edited

Ste1io commented Oct 20, 2023 •

edited