Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-135852: Remove out of tree pywin32 dependency for NTEventLogHandler#137860

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
serhiy-storchaka merged 29 commits intopython:mainfromaisk:nteventlog
Dec 31, 2025

Conversation

@aisk
Copy link
Contributor

@aiskaisk commentedAug 16, 2025•
edited
Loading

Please note that thelpUserSid parameter inReportEvent don't added because it needs passing a new struct from Python to it, and we don't need this parameter for the usage of implementNTEventLogHandler. If we want add it in the future, we can add it as kwargs.

Docuements for these functions:

For theNTEventLogHandler, there is a implementation detail:

The old implementation based onpywin32, ifdllname is not specified,pywin32 will using a vendored dll in that package, as a log template for the NT event log system.

For the new implement in this PR, if thedllname is not specified, I decided not to register the dll. As a result, the log still can be send to the NT Event Log System, we can see it in the Event Viewer.

But the event will show some thing like this (I just use a picture from the Internet because I'm using a localized Windows which the text is Chinese but they are the same thing):

3dcfe02f31b83fb2ddbe167ee871f91278a8885c_2_571x500

And I think it's OK because we can check the Detail tab to see the original message, for develop purpose this is enough. For users with production usage, they can provide their own dlls as the message template.

{2AD0DB7D-2BDC-4C4F-ABFF-064BD3787642}

Copy link
Contributor

@sharktidesharktide left a comment

Choose a reason for hiding this comment

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

Please fix the test fails :)

@aisk
Copy link
ContributorAuthor

Sorry, I forgot to check the CI result after the push.

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Thank you for your PR@aisk. It look interesting.

Could you please show how can this be used inNTEventLogHandler? Maybe not in this PR, but in a separate PR created on top of this?

@aiskaisk marked this pull request as draftSeptember 8, 2025 15:52
@aiskaisk marked this pull request as ready for reviewSeptember 9, 2025 17:27
@aisk
Copy link
ContributorAuthor

aisk commentedSep 9, 2025

Thank you for your PR@aisk. It look interesting.

Could you please show how can this be used inNTEventLogHandler? Maybe not in this PR, but in a separate PR created on top of this?

Thanks for the view! I'll create another PR based on this to try to implement the whole feature in the original issue.

@aiskaisk requested a review fromvsajip as acode ownerOctober 19, 2025 14:39
@aiskaisk changed the titlegh-135852: Add NTEventLog related functions to _winapigh-135852: Remove out of tree pywin32 dependency for NTEventLogHandlerOct 19, 2025
@aisk
Copy link
ContributorAuthor

@serhiy-storchaka Hi, I found that using this inNTEventLogHandler to replacepywin32 is very simple, so I push it directly on this branch, and also added some information on the top of this issue.

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Very well. I have some nitpicks and questions, but mostly LGTM.

NTEventLogHandler only uses a single-string list and don't use a raw_data. Removing these features would simplify the code. But it is fine to keep them for future.

@aisk
Copy link
ContributorAuthor

Hi, thank you for the review! I changed to just to support a single string as event log message to reduce the complexity, and now the codes are ready for review.

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@serhiy-storchakaserhiy-storchaka merged commit3c4429f intopython:mainDec 31, 2025
49 checks passed
@serhiy-storchaka
Copy link
Member

Thank you for your contribution,@aisk. 🎄

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotAMD64 Arch Linux Asan Debug 3.x (no tier) has failed when building commit3c4429f.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/585/builds/7088) and take a look at the build logs.
  4. Check if the failure is related to this commit (3c4429f) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/585/builds/7088

Failed tests:

  • test_math
  • test_statistics
  • test_profiling

Failed subtests:

  • test_process_pool_executor_pickle - test.test_profiling.test_sampling_profiler.test_advanced.TestProcessPoolExecutorSupport.test_process_pool_executor_pickle

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.asan_debug/build/Lib/test/test_profiling/test_sampling_profiler/test_advanced.py", line237, intest_process_pool_executor_pickleself.assertIn("Results: [2, 4, 6]", stdout)~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^AssertionError:'Results: [2, 4, 6]' not found in ''

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka approved these changes

@vsajipvsajipAwaiting requested review from vsajipvsajip is a code owner

+1 more reviewer

@sharktidesharktidesharktide left review comments

Reviewers whose approvals may not affect merge requirements

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

@aisk@serhiy-storchaka@bedevere-bot@sharktide
[8]ページ先頭
©2009-2026 Movatter.jp