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

handle UNC paths for windows #16005

Merged
merged 4 commits into from
Nov 14, 2024
Merged

handle UNC paths for windows #16005

merged 4 commits into from
Nov 14, 2024

Conversation

zzstoatzz
Copy link
Collaborator

@zzstoatzz zzstoatzz commented Nov 13, 2024

closes #15971

I'm happy to remove this new CI job I used to repro, but it wont run unless we change the filesystem utils anyways

@github-actions github-actions bot added the bug Something isn't working label Nov 13, 2024
Copy link

codspeed-hq bot commented Nov 13, 2024

CodSpeed Performance Report

Merging #16005 will not alter performance

Comparing path-to-unc (a35f3cb) with main (8758666)

Summary

✅ 3 untouched benchmarks

@zzstoatzz zzstoatzz self-assigned this Nov 13, 2024
@zzstoatzz zzstoatzz added the arch:windows Related to the Windows OS label Nov 13, 2024
@zzstoatzz zzstoatzz marked this pull request as ready for review November 13, 2024 20:18
@zzstoatzz zzstoatzz changed the title handle unc handle UNC paths for windows Nov 13, 2024
Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

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

Makes sense to me; I'm cool with the new job too given the filters you applied - I really like that approach

@cicdw
Copy link
Member

cicdw commented Nov 13, 2024

@zzstoatzz sorry just thinking through the workflow filters again -- do you think we benefit if it runs on core dependency changes (requirements.txt and requirements-client.txt) as well?

@zzstoatzz
Copy link
Collaborator Author

@zzstoatzz sorry just thinking through the workflow filters again -- do you think we benefit if it runs on core dependency changes (requirements.txt and requirements-client.txt) as well?

to be safe I think that'd make sense, considering its not an expensive CI job. added in 591c3b5

@zzstoatzz zzstoatzz removed the arch:windows Related to the Windows OS label Nov 14, 2024
@zzstoatzz zzstoatzz merged commit 6696911 into main Nov 14, 2024
37 checks passed
@zzstoatzz zzstoatzz deleted the path-to-unc branch November 14, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When trying to run served flow (from_source) from Windows share folder it fails
2 participants