pinterest / elixir-thrift Goto Github PK
View Code? Open in Web Editor NEWA Pure Elixir Thrift Implementation
Home Page: https://hexdocs.pm/thrift/
License: Apache License 2.0
A Pure Elixir Thrift Implementation
Home Page: https://hexdocs.pm/thrift/
License: Apache License 2.0
Our team tends to deprecated thrift fields by prepending DEPRECATED_
to the name -
struct Thing {
1: i32 id,
2: string DEPRECATED_phone_number
}
This generates a struct with an invalid field name: defstruct(id: nil, DEPRECATED_phone_number: nil)
which causes the generated file to fail to compile.
Currently it is not possible to do
Client.start_link("localhost", 33722, [name: __MODULE__])
Right now it generates an entire directory. Instead, we should be able to point it at a single thrift file and it should figure out how to generate that one file and its dependencies.
mix thrift.generate thrift/my_service.thrift
Again I need to come up with an isolated test case or prove to myself that I'm incorrect, but I wanted to document this because I have to switch gears :/ Also this is on thrift_tng
.
I.e.,
enum SomeEnum {
ONE = 1,
TWO = 2
}
struct SomeStruct {
1: optional SomeEnum some_enum
}
is giving me
defmodule SomeStruct do
# ...
defstruct(some_enum: 1)
# ...
end
This is along the same lines as #192 except it wants to allow a list to serialize to a set. This may actually already be supported unintentionally?
Given the following thrift spec:
struct Thang {
1: required i64 thang_id,
2: string thang_name
}
The field thang_name
exhibits "Default requiredness" and does not have to be present. However, our validator blows up and demands that this field be set.
The correct behavior is to allow this field to remain unset, and not serialize it.
The section about Requiredness here http://thrift.apache.org/docs/idl is helpful
Regarding this error handling:
elixir-thrift/lib/thrift/binary/framed/client.ex
Lines 229 to 236 in 1168b29
I believe that a timeout will cause the client to get out of sync and can only ever return an prior sequence id - if it does not keep timing out. This occurs because the next call
will receive the result of the prior call - if it receives anything. This can continue forever with no hope of recovery.
I have not tried this but it appears that by returning an error atom on tcp error it will try to raise an atom later on
, which will fail. Should that use:inet.format_error/1
on the :inet
error atom to raise a RuntimeError or create a custom exception?The Union type has been omitted;
Unions are like structs, but only have one field sent. That means they need to validate themselves at serialize time (or provide setters that clear other fields).
Serialization and deserialization needs to be implemented as well.
(On thrift_tng)
The compile error is
** (Enum.EmptyError) empty error
(elixir) lib/enum.ex:1590: Enum.reduce/2
(mix) lib/mix/utils.ex:80: Mix.Utils.stale_stream/2
(mix) lib/mix/utils.ex:66: Mix.Utils.stale?/2
(elixir) lib/enum.ex:787: anonymous fn/3 in Enum.filter/2
(elixir) lib/enum.ex:1623: Enum."-reduce/3-lists^foldl/2-0-"/3
(elixir) lib/enum.ex:787: Enum.filter/2
lib/mix/tasks/compile.thrift.ex:42: Mix.Tasks.Compile.Thrift.run/1
(mix) lib/mix/task.ex:296: Mix.Task.run_task/3
The error appears to occur in this function in the compile task when targets
comes back as an empty list:
defp stale?(%FileGroup{initial_file: thrift_file} = group, output_dir) do
targets =
group
|> Thrift.Generator.targets
|> Enum.map(&Path.join(output_dir, &1))
Mix.Utils.stale?([thrift_file], targets)
end
The file I'm parsing just contains some const i32
definitions that aren't referenced in the other thrift files anywhere (they get used in applications that use this set of thrift files). If I pass --force
to the compiler, it gets past this stage but then my library fails to compile because the constants are undefined.
I'll try to come up with a test case that exercises this and if I see an easy fix I'll make a PR.
Updating to the most recent thrift_tng commit breaks some of our existing (working) code. The following test case is distilled down from something in our working thrift IDL files, so if the test is incorrect then I've done a bad job trying to simplify.
@thrift_file name: "numbers.thrift", contents: """
enum Number {
ZERO = 0,
ONE = 1,
TWO = 2,
THREE = 3
}
"""
@thrift_file name: "includes_enum.thrift", contents: """
include "numbers.thrift"
struct Person {
1: optional Number favoriteNumber = Number.ZERO
}
"""
thrift_test "including an emum" do
assert %Person{farvorite_number: 0} == %Person{}
end
Results in
elixir-thrift - thrift_tng! ❯ mix test test/thrift/generator/binary_protocol_test.exs
Excluding tags: [pending: true]
** (RuntimeError) Fatal error: Could not find value: Number.ZERO
(thrift) lib/thrift/generator/utils.ex:279: Thrift.Generator.Utils.resolution_failed/1
(thrift) lib/thrift/generator/utils.ex:186: Thrift.Generator.Utils.quote_value/3
(thrift) lib/thrift/generator/struct_generator.ex:16: anonymous fn/2 in Thrift.Generator.StructGenerator.generate/4
(elixir) lib/enum.ex:1229: Enum."-map/2-lists^map/1-0-"/2
(thrift) lib/thrift/generator/struct_generator.ex:14: Thrift.Generator.StructGenerator.generate/4
(thrift) lib/thrift/generator.ex:181: anonymous fn/3 in Thrift.Generator.generate_struct_modules/1
(stdlib) lists.erl:1263: :lists.foldl/3
(thrift) lib/thrift/generator.ex:179: Thrift.Generator.generate_struct_modules/1
(thrift) lib/thrift/generator.ex:75: Thrift.Generator.generate_schema/1
(thrift) lib/thrift/generator.ex:52: anonymous fn/3 in Thrift.Generator.generate!/2
(elixir) lib/enum.ex:958: anonymous fn/3 in Enum.flat_map/2
(stdlib) lists.erl:1263: :lists.foldl/3
(elixir) lib/enum.ex:1772: Enum.flat_map/2
(elixir) lib/enum.ex:966: Enum.flat_map_list/2
(elixir) lib/enum.ex:967: Enum.flat_map_list/2
(thrift) test/support/lib/thrift_test_case.ex:97: ThriftTestCase.generate_elixir_files/3
(thrift) test/support/lib/thrift_test_case.ex:84: ThriftTestCase.generate_files/4
(thrift) test/support/lib/thrift_test_case.ex:36: ThriftTestCase.quoted_contents/2
test/thrift/generator/binary_protocol_test.exs:658: (module)
When I try to run a specific test in my project by name, the compile.thrift gets invoked on the test file. The first line of my test output is then a parse error, where it's trying to parse the Elixir source code as Thrift.
$ mix test test/whatever.exs
Failed to parse test/whatever_test.exs: Error parsing thrift file test/whatever_test.exs on line 6: {:illegal, '@'}
<test continues to run...>
I tried modifying the run method to raise an error
~/code/plex (master) $ mix test test/whatever_test.exs
==> thrift
Compiling 1 file (.ex)
** (RuntimeError) boom, args = ["test/whatever_test.exs"]
lib/mix/tasks/compile.thrift.ex:38: Mix.Tasks.Compile.Thrift.run/1
(mix) lib/mix/task.ex:296: Mix.Task.run_task/3
(elixir) lib/enum.ex:1184: Enum."-map/2-lists^map/1-0-"/2
(mix) lib/mix/tasks/compile.all.ex:19: anonymous fn/1 in Mix.Tasks.Compile.All.run/1
(mix) lib/mix/tasks/compile.all.ex:37: Mix.Tasks.Compile.All.with_logger_app/1
(mix) lib/mix/task.ex:296: Mix.Task.run_task/3
(mix) lib/mix/tasks/compile.ex:85: Mix.Tasks.Compile.run/1
(mix) lib/mix/task.ex:296: Mix.Task.run_task/3
Somehow compile.thrift gives up on extra options causing the mix test to fail when it is embedded in another application.
Removing this from mix solves the problem, but isn't a permanent solution.
compilers: [:thrift | Mix.compilers],
Log.
>mix test
==> snappyex
could not compile dependency :snappyex, "mix compile" failed. You can recompile this dependency with "mix deps.compile snappyex", update it with "mix deps.update snappyex" or clean it with "mix deps.clean snappyex"
==> ecto
** (Mix) Could not invoke task "compile.thrift": 4 errors found!
--no-deps : Unknown option
--no-archives-check : Unknown option
--no-elixir-version-check : Unknown option
--no-warnings-as-errors : Unknown option
There's an undefined reference to CompactProtocol in the thrift generators.
Apparently, in thrift, this is legal:
struct ILoveC {
1: bool c_is_the_best = 0,
2: bool ones_are_truthy = 1,
}
Our thrift validator complains that these fields aren't true, false or nil.
I talked with @pguillory, and we think that converting these into true or false makes the most sense.
For services, the compile.thrift command generates a behavior, then it's up to the user to create a handler module that implements that behavior. It might be useful to have another command that generates a skeleton implementation of such a handler. This would help reduce the friction to get started in situations where the user has an existing thrift and wants to implement a server in Elixir.
If you had a file:
# foo.thrift
namespace GeneratedThriftStuff
service MyService {
void set(1: binary key, 2: binary value)
string get(1: binary key)
}
You could type:
mix thrift.skeleton --file foo.thrift --service MyService --name MyServiceHandler
And it would write a file:
# lib/my_service_handler.ex
defmodule MyServiceHandler do
@behaviour GeneratedThriftStuff.MyService.Handler
@spec set(binary, binary) :: :ok
def set(key, value) do
raise "Not implemented"
end
@spec get(binary) :: binary
def get(key) do
raise "Not implemented"
end
end
Right now thrift_files
and thrift_output
have to go right in the project config's root namespace. I think it would be cleaner if they were nested under a thrift
key:
def project do
# ..
thrift: [
thrift_files: Path.wildcard("thrift/*.thrift"),
thrift_output: "gen-lib"
]
end
Doing this it might make sense to change the names of the parameters to not have a redundant "thrift_" prefix.
By way of prior art: This is the way dialyxir works, and it's fairly intuitive and unobtrusive.
I need to isolate a test case for this (or prove to myself that I am incorrect) but I'm seeing a double field serialized with an integer value.
Thrift.Test.UserService.Servers.Binary.Framed should be Thrift.Test.UserService.Binary.Framed.Server
Thrift.Test.UserService.Clients.Binary.Framed should be Thrift.Test.UserService.Binary.Framed.Client
I came across this trying to do something like the following.
serialized = BusinessObject.serialize(%BusinessObject{})
deserialized = BusinessObject.deserialize(serialized)
The second line produces :error
because serialized
is iodata and deserialize
expects a binary.
This is a little bit of a contrived use case, but I imagine there are other cases where one might be feeding iodata into the deserializer?
This raises another question, as well. Should there be a way to handle partial messages? I can imagine cases where a server is listening to a data stream, and due to the way that the data is chunked one may receive a partial message. I imagine the API would look something like updated_object = Mod.deserialize(new_data, partial_object_from_previous_data)
Example thrift:
const string DefaultRootPassword = "abc123"
struct BestPracticeSecurity {
1: optional string root_password = DefaultRootPassword
}
Results in
** (MatchError) no match of right hand side value: [:DefaultRootPassword]
(thrift) lib/thrift/parser/models.ex:105: Thrift.Parser.Models.TEnumValue.new/1
(thrift) src/thrift_parser.yrl:132: :thrift_parser.yeccpars2_76/7
(thrift) /usr/local/Cellar/erlang/19.1/lib/erlang/lib/parsetools-2.1.3/include/yeccpre.hrl:57: :thrift_parser.yeccpars0/5
(thrift) lib/thrift/parser/parsed_file.ex:12: Thrift.Parser.ParsedFile.new/1
(thrift) lib/thrift/parser.ex:61: Thrift.Parser.parse_file/1
(thrift) lib/thrift/generator.ex:39: Thrift.Generator.generate!/2
(elixir) lib/enum.ex:925: anonymous fn/3 in Enum.flat_map/2
(elixir) lib/enum.ex:1623: Enum."-reduce/3-lists^foldl/2-0-"/3
(elixir) lib/enum.ex:924: Enum.flat_map/2
(thrift) test/support/lib/thrift_test_case.ex:47: ThriftTestCase.compile_and_build_erlang_helpers/2
(thrift) expanding macro: ThriftTestCase.__before_compile__/1
test/generator/models_test.exs:1: Thrift.Generator.ModelsTest (module)
(elixir) lib/code.ex:363: Code.require_file/2
(elixir) lib/kernel/parallel_require.ex:57: anonymous fn/2 in Kernel.ParallelRequire.spawn_requires/5
This might not be an actual issue, but I wanted to document it just in case.
It appears that when Thrift.Parser.parse/1
comes across a typedef or enum field, it treats that as a Thrift.Parser.Models.StructRef
. In retrospect this is probably because the parser doesn't know about other modules, so it can't do any better than to say it must be a reference to some other type. Having "Struct" in the name threw me off at first, I think.
Here's an example thrift IDL file:
<script src="https://gist.github.com/dantswain/623215759d3413a1a6b7e68b37b039f8.js"></script>When I parse that, the typedefed field and the enum field come out as StructRefs:
iex(4)> Enum.at(idl.structs[:SimpleStruct].fields, 3)
%Thrift.Parser.Models.Field{default: nil, id: 4, name: :bigint,
required: :default,
type: %Thrift.Parser.Models.StructRef{referenced_type: :UgeInt}}
iex(5)> Enum.at(idl.structs[:SimpleStruct].fields, 7)
%Thrift.Parser.Models.Field{default: nil, id: 8, name: :taco_pref,
required: :default,
type: %Thrift.Parser.Models.StructRef{referenced_type: :TacoType}}
I'm writing some code for thrash that will parse a collection of idl files and return a merged IDL. I think to really determine the types for those fields I'm going to have to resolve the references myself? If that code could be useful here I could make it something that would be easy to migrate.
Travis tests fail maybe 30% of the time (guesstimate) and it's always some kind of timeout that goes away with a rebuild. @scohen I think this is in the service / behaviour code and smells like a synchronization problem. If you have some idea what might be failing, I have some tricks up my sleeve to deal with that kind of thing.
Using namespace elixir MyStuff
in the .thrift files produces a pile of warnings along the lines of
[WARNING:/some/path/my_struct.thrift:6] No generator named 'elixir' could be found!
whenever running the official thrift compiler to generate code for another language.
This can be disabled by using -nowarn
but that suppresses all warnings, which seems like a bad idea.
I think supporting the namespace
keyword is fine, but there's something wonky to me about requiring people to opt in to a pile of warnings in their pre-existing workflow. One way to get around this would be to make the namespace
keyword optional and support a namespace option in the compile step configuration (see #194 - thrift: [namespace: MyStuff, input_files: ...]
).
This is a little off-the-walll, but in the Ruby thrift lib, enum values get printed out as their name, which can be pretty nice for debugging purposes. We could theoretically generate a defimpl Inspect
for structs and have it do this. For the most part I think this would just hijack the normal struct implementation but it could replace the enum entries with something like user_status: 1 (UserStatus.active)
.
OTOH it could be not worth the complexity. Something opt-in might be a compromise? I.e., the generated module could include a prettyprint function or something and then we provide docs showing how you could roll your own defimpl
around that.
On my travis tests, some elixir thrift calls takes more than 5000ms so it fails. It would make my tests pass if the timeout could be increased.
Document how to set timeout.
Example:
Our server-side protocol handler currently only supports :ranch_tcp
, but the overall interface is capable of supporting other :ranch_transport
-conforming modules, such as :ranch_ssl
. Consider implementing support for :ranch_ssl
, too.
https://twitter.github.io/finagle/guide/Protocols.html#mux
We're gradually switching internal services from binary protocol to mux. I'm going to explore adding support. The above link points to the Finagle source as a reference implementation. We also have an internal implementation in Python that I'll work from. Does anyone know of any other good resources?
https://blog.parsable.com/using-human-readable-json-endpoints-with-thrift-for-free-774ba505c893
Any interest in implementing this?
If you don't specify an elixir namespace, the generator puts files into the root namespace. This is troublesome, as different thrift modules can specify the same name for a struct / union / enum.
If no namespace is specified, we should use the thrift namespace as a default.
The Apache Thrift compiler lets you specify a list of paths to search for included files:
-I dir Add a directory to the list of directories
searched for include directives
We should add a similar capability to our generator, probably configured via a thrift_includes: []
value.
Elixir requires exception structs to define two function in compliance with the Exception behaviour: exception/1
and message/1
.
We generate Elixir exception structs via defexception
for Thrift exception types. That macro always provides an exception/1
implementation, but message/1
is only generated if the exception is defined with a :message
field. When it doesn't, the generated code produces warnings like this:
warning: undefined behaviour function message/1 (for behaviour Exception)
message/1 - receives the exception struct and must return its message. Most commonly exceptions have a message field which by default is accessed by this function. However, if an exception does not have a message field, this function must be explicitly implemented.
We can accommodate this requirement in one of two ways:
:message
key into our exception structs whenever one doesn't already exist and give it a reasonable default value.message/1
functions for exceptions, conditionally (based on the existence of a :message
field) or always.(1) is the easiest thing to do in terms of code changes, but it has the disadvantage of introduce a struct field that doesn't exist in the underlying Thrift IDL.
(2) doesn't suffer from any obvious disadvantages, although it does appear to involve more complicated code changes based on how we generate the quoted module AST (unless I'm missing a more obvious way to conditionialize it).
When compiling something like:
struct Binaries {
1: optional map<string, binary> properties;
}
I get:
lib/thrift/generator/struct_binary_protocol.ex:408: Thrift.Generator.StructBinaryProtocol.map_value_deserializer(:binary, :deserialize__properties__key, :deserialize__properties__value, %Thrift.Parser.FileGroup{initial_file: "thrift/graph_service.thrift", ns_mappings: %{graph_service: nil}, parsed_files: %{"graph_service" => %Thrift.Parser.ParsedFile{file_ref: %Thrift.Parser.File
I'm going to wager binaries were omitted.
Assume we have a service called Pinterest.UserService
.
Presently, we generate:
Pinterest.UserService.Server.Framed
Pinterest.UserService.Client.Framed
Pinterst.UserService.Server.Handler.Behaviour
I suggest moving them to
Pinterest.UserService.Handler
Pinterest.UserService.Binary.Framed.Server
Pinterest.UserService.Binary.Framed.Client
Similarly, I'll place the server / client implementations in
Thrift.Impl.Binary.Framed.Client
Thrift.Impl.Binary.Framed.Server
This will make aliases nicer, we can alias Pinterest.UserService.Binary.Framed.Server
and reference it as Server
rather than Framed
.
@dantswain @pguillory @jparise Thoughts?
This is a selfish request, buuuut I have a situation where I need to serialize a very large map. I have guarantees on uniqueness from upstream so it would actually be quite a lot more efficient for me to set the field value to a keyword list and let the serializer use that directly rather than doing kw_list |> Enum.into(%{})
and having the serializer turn that back into a list to enumerate over anyways.
The down sides are a) complexity and b) people could unwittingly break their own stuff if they aren't being careful about uniqueness upstream.
struct Goth {
1: optional list<i32> empty_like_my_soul = []
2: optional set<i32> the_abyss = []
3: optional map<i32, string> going_nowhere = {}
}
results in
Error parsing thrift file <snip> on line 4: syntax error before: ']'
The map case does work. I just added it to my test case to cover "empty container".
The following thrift snippet:
typedef set<i32> SetOfIntegers
struct Thangs {
1: optional SetOfIntegers magic_numbers
}
Causes the following error on mix thrift.generate
:
** (exit) an exception was raised:
** (CaseClauseError) no case clause matching: {:set, :i32}
(thrift) lib/thrift/parser/resolver.ex:37: anonymous fn/2 in Thrift.Parser.Resolver.update/3
(elixir) lib/map.ex:114: Map.do_new_transform/3
(thrift) lib/thrift/parser/resolver.ex:36: Thrift.Parser.Resolver.update/3
(elixir) lib/agent/server.ex:23: Agent.Server.handle_call/3
(stdlib) gen_server.erl:615: :gen_server.try_handle_call/4
(stdlib) gen_server.erl:647: :gen_server.handle_msg/5
(stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3
I think we might have gotten guard-happy
This breaks the parser.
struct ImBadAtInitializing {
1: double myBad = 0;
}
The current thrift.compile logic only recompiles output files based on their last_modified timestamp relative to their respective input files. However they may also need to be regenerated if elixir-thrift itself has changed. For example the client and server have generated user modules interoperating with static modules like Thrift.Client.BinaryFramed. If the latter changes, we need to make sure the former changes as well.
On success, decode returns a tuple with the decoded entity and an empty string. Instead, it would be nice if it did something like:
case MyThriftStruct.BinaryProtocol.decode(binary_data) do
{:ok, %MyThriftStruct{}=s} ->
s
{:more, %MyThriftStruct{}=s, extra_data} ->
...
{:error, reason} ->
...
end
See discussion in #202
The bulk of our CI (Travis) build time is spent (re)building the thrift compiler tool. (There unfortunately isn't a readily available Thrift 0.9.3 .deb package.)
Consider using a Docker-based build package like in dantswain/thrash#8.
When running 2 servers with the same handler, I get this error:
iex(1)> Synapse.Server.Read.start_link
The erlang-history file at /Users/alialtaf/.erlang-hist.nonode@nohost was corrupted by DETS. An attempt to repair the file will be made, but if it fails, history will be ignored for the rest of this session. If the problem persists, please delete the history file (and maybe submit it with a bug report).
17:07:43.334 [error] CRASH REPORT Process <0.432.0> with 0 neighbours crashed with reason: no match of right hand value false in ranch_server:set_connections_sup/2 line 66
17:07:43.334 [error] Supervisor {<0.431.0>,ranch_listener_sup} had child ranch_conns_sup started with ranch_conns_sup:start_link('Elixir.Synapse.Server.Handler', worker, 5000, ranch_tcp, 5000, 'Elixir.Thrift.Binary.Framed.ProtocolHandler') at undefined exit with reason {badmatch,false} in context start_error
17:07:43.334 [error] Supervisor {<0.430.0>,'Elixir.Supervisor.Default'} had child {ranch_listener_sup,'Elixir.Synapse.Server.Handler'} started with ranch_listener_sup:start_link('Elixir.Synapse.Server.Handler', 1, ranch_tcp, [{port,2323}], 'Elixir.Thrift.Binary.Framed.ProtocolHandler', {'Elixir.Thrift.Synapse.Synapse.Binary.Framed.Server','Elixir.Synapse.Server.Handler'}) at undefined exit with reason {shutdown,{failed_to_start_child,ranch_conns_sup,{badmatch,false}}} in context start_error
** (EXIT from #PID<0.425.0>) shutdown: failed to start child: {:ranch_listener_sup, Synapse.Server.Handler}
** (EXIT) shutdown: failed to start child: :ranch_conns_sup
** (EXIT) {:badmatch, false}`
The fix was to specify unique names when calling Server.start_link
Running mix thrift.generate
with a directory that has a bunch of thrift files in it, I get this error:
** (MatchError) no match of right hand side value: {:error, {32, :thrift_parser, ['syntax error before: ', '\']\'']}}
lib/thrift/parser.ex:19: Thrift.Parser.parse/1
lib/thrift/parser/parsed_file.ex:13: Thrift.Parser.ParsedFile.new/1
lib/thrift/parser.ex:57: Thrift.Parser.parse_file/1
lib/thrift/generator.ex:11: Thrift.Generator.generate!/2
lib/mix/tasks/thrift.generate.ex:32: anonymous fn/3 in Mix.Tasks.Thrift.Generate.run/1
(elixir) lib/enum.ex:1623: Enum."-reduce/3-lists^foldl/2-0-"/3
lib/mix/tasks/thrift.generate.ex:31: Mix.Tasks.Thrift.Generate.run/1
(mix) lib/mix/task.ex:296: Mix.Task.run_task/3
I don't think any of my files have thrift syntax errors, which is another issue that I'll try to track down, but this doesn't tell me which file is causing the error.
thrift's i64 decoding may be broken
I get 18446744011573782016 instead of -62135769600
Example code:
<micmus> <<int::integer-64-signed>> = :binary.encode_unsigned(18446744011573782016)
<micmus> int => -62135769600
<iFire> iex(44)> DateTime.from_unix(int)
<iFire> {:ok, #<DateTime(0000-12-30T00:00:00Z Etc/UTC)>}
<iFire> \o/
This means 16, 32 and 64 signed integers are also broken according to a review of struct_binary_protocol.ex line 132
.
Can be reproduced by changing this case in models_test.exs
@thrift_file name: "complex_typedefs.thrift", contents: """
typedef Stuff i32
typedef set<i32> IntSet
struct Thangs {
1: optional IntSet numbers,
2: optional set<Stuff> set_of_stuff
}
"""
Then running mix test results in
** (MatchError) no match of right hand side value: {:error, {2, :thrift_parser, ['syntax error before: ', 'i32']}}
(thrift) lib/thrift/parser.ex:20: Thrift.Parser.parse/1
(thrift) lib/thrift/parser/parsed_file.ex:13: Thrift.Parser.ParsedFile.new/1
(thrift) lib/thrift/parser.ex:60: Thrift.Parser.parse_file/1
(thrift) lib/thrift/generator.ex:11: Thrift.Generator.generate!/2
(elixir) lib/enum.ex:925: anonymous fn/3 in Enum.flat_map/2
(elixir) lib/enum.ex:1623: Enum."-reduce/3-lists^foldl/2-0-"/3
(elixir) lib/enum.ex:924: Enum.flat_map/2
(thrift) test/support/lib/thrift_test_case.ex:47: ThriftTestCase.compile_and_build_erlang_helpers/2
(thrift) expanding macro: ThriftTestCase.__before_compile__/1
test/generator/models_test.exs:1: Thrift.Generator.ModelsTest (module)
(elixir) lib/code.ex:363: Code.require_file/2
(elixir) lib/kernel/parallel_require.ex:57: anonymous fn/2 in Kernel.ParallelRequire.spawn_requires/5
I believe the constants-merging code from #224 might be breaking the generation of code that follows this pattern:
a.thrift
defines a bunch of things, including structs and a service.a.thrift
also includes b.thrift
.b.thrift
defines some constants.This results in the top-level module produced by a.thrift
(A
) containing only the constants from b.thrift
.
// a.thrift
include "b.thrift"
struct Foo {
1: string name
}
// b.thrift
const i8 Z = 26
This results in an a.ex
(defmodule(A)
) containing the constant and a foo.ex
containing struct Foo
. I would have expected the constant to be written to a b.ex
(defmodule(B)
) instead.
This is a follow-up to #227 (fixed by #230). The current behavior of the parser/generator is to iterate over the list of input files and do a full parse/generate for each. This leads to a fair amount of double-processing and at times writing out generated code more than once. For the most part this isn't a huge problem because the generated code is consistent, but as we saw with Constants it can lead to modules being clobbered.
On one hand, it seems like the right thing to do might be to iterate over the list of input files (and any files that those include) and parse out one unified schema from which to generate code. This should be the most consistent way to generate output. It seems like it would also require fairly significant changes to the code.
On the other hand, I believe the way we are doing it now is close(r) to the way that most of the official thrift generators work: generally one executes thrift -o whateverlanguage path/to/some.thrift
which parsers a single input file and generates code from that file. I'm not 100% sure whether that will generate structs/enums/etc from files that are include
d from the original file. These parsers don't seem to really go out of their way to make sure that developers don't clobber their own generated code due to name collisions. I'm also not sure that they go out of their way to make sure that any code you generate will be complete from a name resolution standpoint (i.e., if you reference Foo
, it may not always care if you've processed the file that generates Foo
).
An intermediate solution would be to only generate code for the "initial file" in the file group. I.e., if file x.thrift
does include "y.thrift"
which defines struct Y
, then we parse y.thrift
to resolve references but do not generate Y
except when specifically processing y.thrift
.
I guess a first step here would be to do some research on how the existing parsers work, especially wrt how they handle include
d files - I'm kind of shooting from the cuff based on past experience but I'm not 100% confident.
Say there are two versions of a thrift spec. One has a field and one doesn't. If we generate models on the spec without the field, it won't deserialize a message that contains the field.
The correct behavior is skipping that field.
I don't know if it's easy to fix this and it shouldn't hold us back I don't think, but I thought I'd make an issue for discussion's sake (maybe someone has an idea for a quick fix). The thrift_test
macro makes it impossible to run, e.g., mix test test/thrift/generator/binary_protocol_test.exs:599
because the line numbers get lost. Maybe it's as simple as adding location: :keep
to the defmacro
?
When using elixir-thrift, I found myself doing a lot of this:
defmodule UsingThrift do
alias Thrift.Service.MyEnum
alias Thrift.Service.MyOtherEnum
require MyEnum
require MyOtherEnum
end
I was thinking that it would be nice if we'd have something that would do the require of all the enums for us so I could do
defmodule ThriftUser do
use Thrift.Service
alias Thrift.Service.{MyEnum, MyOtherEnum}
end
or
defmodule ThriftUser do
use Thrift.Service
end
I don't have opinions about much here, I just want to spur conversations and improve the ergonomics of our enums.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.