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: port over initial python schema extraction PoC #3332

Merged
merged 12 commits into from
Nov 6, 2024

Conversation

mistermoe
Copy link
Member

@mistermoe mistermoe commented Nov 6, 2024

Summary

This PR ports python schema extraction PoC into this repo

Warning

Still mostly unusable but this lays in the vast majority of a foundation.

Changes

  • refactor verb decorator. switched to using a pattern that doesn't require dynamic dangling mutable properties on functions. Instead encapsulates decorated verb functions in a class and extracts type information useful for runtime grpc server and schema extraction
  • sets up initial stab at a directory structure. will attempt to create 1 package per schema feature. each package will contain:
    • encapsulating class
    • decorator
    • schema extraction
  • minor refactor of VerbExtractor to use isinstance(func, Verb) instead of getattr(func, "_is_ftl_verb", False).
  • set up runnable schema_extractor (note: this will change in the next PR to a standalone executable. temporarily in a cli dir)
  • added TODO in python language plugin's Build method

props to @worstell for doing the actual schema extraction work! will co-author the merge commit

Usage

Note

From the python-runtime/ftl dir

❯ uv run python -m ftl.cli.schema_extractor $(pwd)/../../examples/python/echo
Error importing module decorator: attempted relative import with no known parent package
/Users/moe/code/tbd/ose/ftl/python-runtime/ftl/src/ftl/extract/context.py:76: UserWarning: google.protobuf.service module is deprecated. RPC implementations should provide code generator plugins which generate code specific to the RPC implementation. service.py will be removed in Jan 2025
  spec.loader.exec_module(module)
Extracted Decl:
pos {
  filename: "/path/to/ftl/python-runtime/ftl/../../examples/python/echo/echo.py"
  line: 17
}
name: "echo"
request {
  ref {
    name: "EchoRequest"
    module: "echo"
  }
}
response {
  ref {
    name: "EchoResponse"
    module: "echo"
  }
}

Extracted Decl:
pos {
  filename: "/path/to/ftl/python-runtime/ftl/../../examples/python/echo/echo.py"
  line: 7
}
name: "EchoRequest"
fields {
  name: "name"
  type {
    string {
    }
  }
}

Extracted Decl:
pos {
  filename: "/path/to/ftl/python-runtime/ftl/../../examples/python/echo/echo.py"
  line: 12
}
name: "EchoResponse"
fields {
  name: "message"
  type {
    string {
    }
  }
}

@ftl-robot ftl-robot mentioned this pull request Nov 6, 2024
@@ -9,6 +9,7 @@ authors = [
requires-python = ">=3.12"
dependencies = [
"protobuf>=5.28.3",
"google>=3.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't seem necessary? No source links on the project page either.

Python bindings to the Google search engine.

Copy link
Member Author

Choose a reason for hiding this comment

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

nope good catch. artifact from poc. removed it

print(f"Extracted Decl:\n{decl}")


def analyze_file(global_ctx: GlobalExtractionContext, file_path, analyzer_batch):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing annotations here and elsewhere?

I'd like it if all the Python code were fully typed, and linted for typing too?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep agreed! just configured the linter to require type annotations for everything: e8b9bf2

will go through and annotate everything now


__all__ = [
"extract_type",
"extract_slice",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do these other functions need to be exported?

return None


def extract_slice(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe these should be _private?

def extract_class_type(
local_ctx: LocalExtractionContext, type_hint: Type[Any]
) -> Optional[schemapb.Type]:
ref = schemapb.Ref(name=type_hint.__name__, module=type_hint.__module__)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure what state this is in, so maybe this is a bit premature, but using __module__ isn't going to be sufficient IIRC?

Copy link
Member Author

Choose a reason for hiding this comment

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

aye chatted with @worstell , probably not going to be sufficient. will leave this to tackle in a subsequent PR

python-runtime/ftl/src/ftl/extract/context.py Outdated Show resolved Hide resolved
python-runtime/ftl/src/ftl/extract/context.py Outdated Show resolved Hide resolved
python-runtime/ftl/src/ftl/extract/context.py Outdated Show resolved Hide resolved
@mistermoe mistermoe merged commit d3c73f5 into main Nov 6, 2024
94 checks passed
@mistermoe mistermoe deleted the moe/py-schema-extraction branch November 6, 2024 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants