Skip to content
This repository was archived by the owner on Mar 8, 2020. It is now read-only.

Add the initial module#1

Merged
smola merged 8 commits intobblfsh:masterfrom
vmarkovtsev:master
Jun 15, 2017
Merged

Add the initial module#1
smola merged 8 commits intobblfsh:masterfrom
vmarkovtsev:master

Conversation

@vmarkovtsev
Copy link
Contributor

It is capable of fetching UASTs

@vmarkovtsev vmarkovtsev force-pushed the master branch 2 times, most recently from 8261434 to 137c429 Compare June 14, 2017 16:19
It is capable of fetching UASTs
self._channel = grpc.insecure_channel(endpoint)
self._stub = ProtocolServiceStub(self._channel)

def fetch_uast(self, file_path, language, contents=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that language could also be None by default for autodetection.

help="bblfsh gRPC endpoint.")
parser.add_argument("-f", "--file", required=True,
help="File to parse.")
parser.add_argument("-l", "--language", required=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be optional for autodetection

-I github.com/bblfsh/sdk/protocol -I $(makefile_dir) \
github.com/bblfsh/sdk/protocol/generated.proto

%/__init__.py:
Copy link
Contributor

@juanjux juanjux Jun 14, 2017

Choose a reason for hiding this comment

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

(non blocking) another option would be to add a first level __init__ with an import hook that knows how to do the rest, but this does the job perfectly too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is very hacky too.

Copy link
Contributor

@juanjux juanjux Jun 14, 2017

Choose a reason for hiding this comment

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

It is but done well it could be more generic/automatic for other use cases of this gRPC module.

bblfsh/test.py Outdated
self.client = BblfshClient("0.0.0.0:9432")

def testUAST(self):
uast = self.client.fetch_uast(__file__, "Python")
Copy link
Contributor

@juanjux juanjux Jun 14, 2017

Choose a reason for hiding this comment

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

uast in this case is a response object more than a UAST that would be part of it, I'm right? In that case it should probably check for errors in the response (response.errors and/or response.status).

bblfsh/test.py Outdated
@classmethod
def setUpClass(cls):
subprocess.check_call(
"docker run --privileged -p 9432:9432 --name bblfsh -d "
Copy link
Contributor

@juanjux juanjux Jun 14, 2017

Choose a reason for hiding this comment

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

Ideally we would have to mock the server to avoid this external dependency on the test, but this should serve for now.

@vmarkovtsev
Copy link
Contributor Author

Update: now it launches the Babelfish server if it is not running (can be disabled).

bblfsh/client.py Outdated
self._channel = grpc.insecure_channel(endpoint)
self._stub = ProtocolServiceStub(self._channel)

def fetch_uast(self, file_path, language=None, contents=None):
Copy link
Member

Choose a reason for hiding this comment

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

Please, call this parse_uast so that it matches the protocol as much as possible.

Copy link
Member

Choose a reason for hiding this comment

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

Also rename file_path to filename so it matches too.

from bblfsh.github.com.bblfsh.sdk.protocol.generated_pb2_grpc import ProtocolServiceStub


class BblfshClient(object):
Copy link
Member

Choose a reason for hiding this comment

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

I think this should have a method to close the client and the underlying connection.
Maybe close()? I'm not sure that's Python's convention. Also, __enter__ and __exit__ so it works with with?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no close(). Python does it automatically without ResourceWarning-s. Even the Python grpc primer does not close anything: http://www.grpc.io/docs/tutorials/basic/python.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

👍

@vmarkovtsev
Copy link
Contributor Author

@smola Fixed!

@smola smola merged commit 0e8ca2e into bblfsh:master Jun 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants