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

fix(win): fix plugin startup / use FileAppender #45

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

YOU54F
Copy link
Contributor

@YOU54F YOU54F commented Aug 21, 2024

issues on reading the plugin startup message when using system threads over tokio tasks, seem to be due to the RollingFileAppender

Built locally on a mac m1 (via docker) and tested in a windows VM using parallels

Plugin now start ups correctly, and maintains logs. fixes #42

Using the pact-plugin-driver with this change

https://github.com/pact-foundation/pact-plugins/pull/69/files#diff-e21b8ae9cb59f07bada083f39629f6fbc221a1c6ca0ad395799d0bad5f7bc941R290

in order to terminate child processes on windows, will fix the shut down issues.

Tested e2e with the avro plugin example in https://github.com/pact-foundation/pact-go/tree/master/examples/avro

@YOU54F
Copy link
Contributor Author

YOU54F commented Aug 22, 2024

Tested this out x-plat, in dotnet & pact-go. fixes the startup issues.

Shutdown issues on windows resolved by pact-foundation/pact-plugins#69

specifically this line https://github.com/pact-foundation/pact-plugins/pull/69/files#diff-0d68f182064e209fefba06eec9ce538b69dc667ef8fcba52143b0778cdb7b3ebR122

⚠️ Once the above PR is merged and released in the Pact FFI library, it means that any pact-avro-plugin usage prior to this change being applied, will cease to work (not just on windows, but on Linux/MacOS too), and therefore consumers of this library should be urged to update.

If they update to the latest version of the pact-avro-plugin that includes this release, it will work with the current FFI, and later versions of the FFI, consuming the above pact-plugin change.

So don't delay, update today 😅

<file>logs/plugin.log</file>
<rollingPolicy class="ch.qos.logback.core.rolling.TimeBasedRollingPolicy">
<!-- daily rollover -->
<!-- <rollingPolicy class="ch.qos.logback.core.rolling.TimeBasedRollingPolicy">
Copy link
Owner

Choose a reason for hiding this comment

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

as the RollingFileAppender is not used anymore, there is no need to keep the settings for it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed now 👍🏾

issues on reading the plugin startup message when using system threads
over tokio tasks, seem to be due to the RollingFileAppender
@YOU54F YOU54F force-pushed the fix/windows_startup branch from 2516e3e to aee1662 Compare August 30, 2024 11:32
@austek austek merged commit 22a3cda into austek:main Aug 30, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants