Skip to content

BE: issues/711: Normalize path for proto schemas locations for Windows OS #712

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.

Sign up for GitHub

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

Closed
DimaVilda wants to merge 2 commits into from
Closed

BE: issues/711: Normalize path for proto schemas locations for Windows OS #712

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78cf18d
issues/711: normalize path for proto schemas locations for Windows OS
DimaVilda Dec 15, 2024
f35ca5a
Merge branch 'main' into issues/711-fix-npe-on-tests-on-windows
DimaVilda Dec 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion api/src/main/java/io/kafbat/ui/serdes/builtin/ProtobufFileSerde.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,10 @@ private Map<String, ProtoFile> loadFilesWithLocations() {
// relative path will be used as "import" statement
String relativePath = baseLocation.relativize(path).toString();
var protoFileElement = ProtoParser.Companion.parse(
Location.get(baseLocation.toString(), relativePath),
Location.get(

@Haarolean Haarolean Dec 15, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for the PR.
This is what I meant in #696 (comment). This feels like a hacky solution aiming to fix the tests on Windows. In the end, there are built-in APIs in io/nio that we'd like to use if there's a need.
it's been discussed here before: #261 (comment)
Considering this is a bug in wire, I'd wait for it to get fixed so we don't have to resort to such solutions.

Let me know what you think

@DimaVilda DimaVilda Dec 15, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, we can wait for some fix from 'wire' side and I understnand that it looks hacky, that is why I higlighted this approach as tempo in comments. Feel free to close it.

baseLocation.toString().replace('\\', '/'),
relativePath.replace('\\', '/')
),
readFileAsString(path)
);
filesByLocations.put(relativePath, ProtoFile.Companion.get(protoFileElement));
Expand Down