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

Port engine #6133

Merged
merged 6 commits into from Jul 18, 2018

Conversation

Projects
None yet
2 participants
@Eric-Arellano
Copy link
Contributor

Eric-Arellano commented Jul 15, 2018

Part of #6062.

@@ -156,7 +157,7 @@ def _root_type_ids(self):

def graph_trace(self, execution_request):
with temporary_file_path() as path:
self._native.lib.graph_trace(self._scheduler, execution_request, bytes(path))
self._native.lib.graph_trace(self._scheduler, execution_request, bytes(path, 'utf-8'))

This comment has been minimized.

@Eric-Arellano

Eric-Arellano Jul 15, 2018

Contributor

I'm assuming utf-8 is the correct encoding. It works with the tests, and I don't see any reason for us to use something else like ascii, but do want to flag this file.

This comment has been minimized.

@stuhood

stuhood Jul 16, 2018

Member

Yea, it should be. On osx and linux, paths are utf8.

@stuhood
Copy link
Member

stuhood left a comment

Thanks... looks good.

The rust tests should not actually be affected by these changes, as they don't depend on the python code at all. Did a rust test flake maybe?

@@ -156,7 +157,7 @@ def _root_type_ids(self):

def graph_trace(self, execution_request):
with temporary_file_path() as path:
self._native.lib.graph_trace(self._scheduler, execution_request, bytes(path))
self._native.lib.graph_trace(self._scheduler, execution_request, bytes(path, 'utf-8'))

This comment has been minimized.

@stuhood

stuhood Jul 16, 2018

Member

Yea, it should be. On osx and linux, paths are utf8.

@Eric-Arellano

This comment has been minimized.

Copy link
Contributor

Eric-Arellano commented Jul 16, 2018

Oh that's good! I'll rerun the full test suite then - I was bummed thinking I would have to debug Rust code 😂

@stuhood stuhood merged commit 2bf15d4 into pantsbuild:master Jul 18, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Eric-Arellano Eric-Arellano deleted the Eric-Arellano:port-engine branch Jul 18, 2018

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