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

feat: add universal fix to allow python detection on DLLs #4538

Merged
merged 6 commits into from
Dec 18, 2024

Conversation

jananir640
Copy link
Contributor

In a previous PR (#4023), I added a fix to allow for python3.11 detection on python DLLs, but this did not hold for python 3.12 detection. Here, I am modifying the version pattern to be more applicable across various python DLLs.

More detail: There's a string tags/v3.xx.xx that is followed by the string version_info in 3.11, and the same string tags/v3.xx.xx exists in the 3.12 DLL but is followed by the string unraisablehook. So, updating the version pattern to be just r"tags/v([23]+\.[0-9]+\.[0-9]+)\r?\n" instead of r"tags/v([23]+\.[0-9]+\.[0-9]+)\r?\nversion_info" as before. I tested this out, and successfully detected python version 3.12.6.

@jananir640
Copy link
Contributor Author

@terriko Hi Terri! Wanted to reach out because this PR keeps failing some long tests. The only minor change here is modifying a regex string so I'm not sure what is causing these failures. Would you happen to have any insight as to what might be going wrong here? Really appreciate you taking a look, thank you!

@terriko
Copy link
Contributor

terriko commented Nov 4, 2024

Yeah, something is definitely up with the long tests. Usually that kind of failure is transient so I was letting it sit for a couple of days, but if it's still happening this week then I'm going to need to switch how those are run. It's freezing halfway through the tests and I suspect since that one job (and only that one job) is being run on free-tier github actions at the moment, that may be the problem.

@terriko
Copy link
Contributor

terriko commented Nov 12, 2024

Updating the branch to get the longtests to run with the new config.

@jananir640
Copy link
Contributor Author

Wanted to follow up on this--seems like the new configuration didn't help with the failing longtest...is there something else we can try? Thanks in advance!

@terriko
Copy link
Contributor

terriko commented Dec 4, 2024

I've filed an issue about what I think is going on with the longtests: #4602

I doubt I'm going to get it resolved today, but I'll send some questions out to the team that's handling our other runners and if they don't have an easy solution, I've scheduled some time on Friday to see if I find a different resolution. Sorry it's taking so long -- I got pulled into another project and the person who normally covers for me here is out on leave.

@jananir640
Copy link
Contributor Author

No problem, thank you for all your help!

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Okay, I think we've resolved the long test issues. I'm going to go ahead and update the branch which will re-run all the tests. I'll set this up to merge once they pass. Thank you again for your work on this and your incredible patience!

@terriko terriko enabled auto-merge (squash) December 18, 2024 18:45
@terriko terriko merged commit d389887 into intel:main Dec 18, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants