Skip to content

#984 single file publish works with Service connectors#1237

Merged
TimHess merged 7 commits into
SteeltoeOSS:release/3.2from
thompson-tomo:fixes/#984_SingleFilePublish
Jan 24, 2024
Merged

#984 single file publish works with Service connectors#1237
TimHess merged 7 commits into
SteeltoeOSS:release/3.2from
thompson-tomo:fixes/#984_SingleFilePublish

Conversation

@thompson-tomo
Copy link
Copy Markdown
Contributor

@thompson-tomo thompson-tomo commented Dec 28, 2023

Description

This fix enables dotnet application's published as a single file to work which would previously fail due to an issue loading files

Fixes #984

Quality checklist

  • Your code complies with our Coding Style.
  • You've updated unit and/or integration tests for your change, where applicable.
  • You've updated documentation for your change, where applicable.
    If your change affects other repositories, such as Documentation, Samples and/or MainSite, add linked PRs here.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.
  • You've added required license files and/or file headers (explaining where the code came from with proper attribution), where code is copied from StackOverflow, a blog, or OSS.

@thompson-tomo thompson-tomo changed the base branch from main to release/3.2 December 28, 2023 07:34
@thompson-tomo thompson-tomo force-pushed the fixes/#984_SingleFilePublish branch 2 times, most recently from 72ed9c2 to bca20f2 Compare December 28, 2023 08:12
@thompson-tomo thompson-tomo force-pushed the fixes/#984_SingleFilePublish branch from bca20f2 to 8d61703 Compare December 28, 2023 08:13
@thompson-tomo
Copy link
Copy Markdown
Contributor Author

@dotnet-policy-service agree

@thompson-tomo thompson-tomo force-pushed the fixes/#984_SingleFilePublish branch from 5cbc34b to b96c399 Compare December 28, 2023 08:24
@TimHess TimHess added Component/Connectors Issues related to Steeltoe connectors ReleaseLine/3.x Identified as a feature/fix for the 3.x release line labels Jan 2, 2024
@TimHess
Copy link
Copy Markdown
Member

TimHess commented Jan 12, 2024

Hey @thompson-tomo,

Thanks for digging into this, I see what you're doing here and I see how it allows the application to continue when published as a single file, but I'm trying to understand if this should be the right behavior.

Swallowing the exception is fine if you're not really dependent on whatever Steeltoe feature is doing the assembly scan (in addition to your Connectors example, I see a reference in Discovery too and I'm not sure if there are others), but it seems like it could also introduce confusion due to the blind spot it could create...

I'm assuming Connectors wouldn't really work after this change - how does your application handle a silent failure here?

@thompson-tomo
Copy link
Copy Markdown
Contributor Author

Hi @TimHess

The example application I included is the shell of my production app where I am encountering the issue. The application works as does the example, my understanding is it is trying to find a dll which doesn't exist due to being published as a single file hence throwing null reference. In the case of single file the library is already loaded.

Copy link
Copy Markdown
Member

@TimHess TimHess 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 including the repro, it can be removed from the PR now as this sample can be used for your scenario as well as service discovery.

I think this will work with a few more changes as noted. I pushed a commit that added and updated helper methods that can ensure the contents of these assemblies are not optimized out at build time

Comment thread src/Common/src/Abstractions/Exceptions/SingleFilePublishedException.cs Outdated
Comment thread src/Common/src/Common/Reflection/ReflectionHelpers.cs Outdated
Comment thread src/Common/src/Common/Reflection/ReflectionHelpers.cs Outdated
@TimHess TimHess added this to the 3.2.7 milestone Jan 17, 2024
@thompson-tomo
Copy link
Copy Markdown
Contributor Author

Thanks @TimHess i will review your comments, test the changes on the weekend & come back to you with my results. Hopefully all is successful.

@thompson-tomo thompson-tomo force-pushed the fixes/#984_SingleFilePublish branch from a2e2539 to ed25a49 Compare January 21, 2024 01:52
@thompson-tomo
Copy link
Copy Markdown
Contributor Author

@TimHess Changes have been made as suggested and all is working as expected in my production app. Let me know if you need anything else from me to get this merged in & it will also need to be ported to v4 release branch.

Copy link
Copy Markdown
Member

@TimHess TimHess left a comment

Choose a reason for hiding this comment

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

LGTM

@TimHess
Copy link
Copy Markdown
Member

TimHess commented Jan 24, 2024

/azp run Steeltoe.All

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@sonarqubecloud
Copy link
Copy Markdown

@TimHess TimHess merged commit 1510dfe into SteeltoeOSS:release/3.2 Jan 24, 2024
@thompson-tomo thompson-tomo deleted the fixes/#984_SingleFilePublish branch January 24, 2024 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component/Connectors Issues related to Steeltoe connectors ReleaseLine/3.x Identified as a feature/fix for the 3.x release line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EFCore Service Connector not usable when published single file

2 participants