Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1526
Did you mean to make this PR? If so, why?
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1516
OK. Looks like the build failures are unrelated. I'm going to apply locally
and check cross-tests just to make sure, but will get this applied today. Thank
you @QuestofIranon! Always happy
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1516
LGTM. @QuestofIranon just curious - why bump the version?
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1269
@jeking3 Yup - just did that. Is there a way to do it automatically, or is
it a manual process (for now)?
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1269
No problem! I'm sorry I was lax and left it so long :/
Looks like the tests passed, so it's good to merge. Not 100% of the
process, so I'll follow up via email.
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1269
@jeking3 Any chance this could be merged? TY!
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1269
@jeking3 I've added unit tests around this behavior and enabled more cross
tests! Do you want me to roll that into this PR, or should I create another bug
and PR?
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1269
@jeking3 Fixed up the failing tests. Confirming that precross works.
---
GitHub user allengeorge opened a pull request:
https://github.com/apache/thrift/pull/1508
THRIFT-4419: Fix bug where framed messages > 4K could not be read
Client: rs
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/allengeo
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1269
Yup. Will do. I'm a gonna set myself a target to get it done by end of next
week. I've been lax on this :/
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1475
On the face of it that looks correct. There isn't even a `message` in
scope, so when it hits that line the code would just break. I'll look a little
deeper.
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1458
@jeking3 Looks like everything passed, so this patch is good to go. TY!
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1458
@jeking3 Thank you!
So...updated the PR to re-add the three failing tests. Created
[THRIFT-4451](https://issues.apache.org/jira/browse/THRIFT-4451) to track this.
I'm going to fix up
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1458
@jeking3 This seems like a separate problem unfortunately. I'd removed
three tests from the "known failures list" and it appears there's a specific
problem related to multiplexed
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1458
@jeking3 This seems like a separate problem unfortunately. I'd removed
three tests from the "known failures list" and it appears there's a specific
problem related to multiplexed
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1458
One of two fixes for problems noticed by others during cross-tests. I've a
separate PR for framed transports.
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1269
Hi James - Iâm really sorry for the delay; slammed at work. Iâll work
on it this weekend.
Again, super sorry about this
From: James E
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1360
@sadikovi Sorry for the delay - was away this weekend.
Ah, ok. That makes total sense. You probably don't have any floats in your
IDL, or anything that would use the `TryFrom
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1360
@sadikovi The change seems fine, but I don't quite understand when this
problem happens. Could you explain when it gets triggered? In what sort of code
setup? Thanks!
---
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1269
@markerickson-wf My bad - I should definitely have run the unit tests. I'll
check them and fix them up. Also, I'll look into how to create a reasonable set
of unit tests for the framed
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1269#discussion_r116517412
--- Diff: lib/dart/lib/src/transport/t_framed_transport.dart ---
@@ -51,33 +58,112 @@ class TFramedTransport extends TBufferedTransport
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1269#discussion_r116517213
--- Diff: lib/dart/lib/src/transport/t_framed_transport.dart ---
@@ -51,33 +58,112 @@ class TFramedTransport extends TBufferedTransport
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1269#discussion_r116516987
--- Diff: lib/dart/lib/src/transport/t_framed_transport.dart ---
@@ -51,33 +58,112 @@ class TFramedTransport extends TBufferedTransport
GitHub user allengeorge opened a pull request:
https://github.com/apache/thrift/pull/1269
THRIFT-4187 Allow dart framed transport to read incomplete frame
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/allengeorge/thrift allen
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1267
@Jens-G Any chance this could go in? Thank you?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
GitHub user allengeorge opened a pull request:
https://github.com/apache/thrift/pull/1267
THRIFT-4196 Support recursive types in Rust
Client: rs
* Modify code generator to support recursive types
* Modify rust test (not cross-test) client to test recursive calls
You
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
@Jens-G Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
@jeking3 Thanks for pointing me in the right direction. Everything now
passes!
I realize you're not committing, but would it be possible for one of the
other maintainers - @Jens-G
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
@jeking3 Oh. Totally didn't realize that - thanks for clarifying! I've
updated my `TMultiplexedProcessor` implementation to have a
backwards-compatibility mode, and am re-running the build
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1252
@jeking3 If you're willing to take this...I have some ruby experience, and
this looks good to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
@Jens-G @jeking3 Can I get any guidance on how the clients/server get their
arguments? I'm confident it's not a bug in the rust server. I can invoke the
c_glib client and rust server manually
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
OK. I really don't understand `tests.json`. I'm confident that my code is
doing the right thing - it's just that the server is invoked in multiplexed
mode and the client is invoked in non
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
Ah. Gotcha. Sorry - I should have realized that. I'll pay closer attention
next time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
I had to split up binary/compact protocols because the builds were timing
out with them together. And, AFAICT, the unit tests aren't running for builds 1
and 2.
Also (I didn't change
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
I've rebalanced the matrix. Trying again.
@jeking3 Having a weird problem with the c_glib client. It's using the
"binary" or "compact" protocol only when invoked
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
@jeking3 One hopes that...congratulations are in order? :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
Please pull the latest commit. I updated to fix a bug in my test server.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1260#discussion_r114546199
--- Diff: test/known_failures_Linux.json ---
@@ -224,5 +224,7 @@
"hs-py3_json_framed-ip",
"java-d_compact_buffered-ip
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
@jeking3 Happy to make the change to the travis jobs! Let me modify this PR
with the defect change and then work on rebalancing the travis jobs.
---
If your project is set up for it, you can
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1260#discussion_r114530042
--- Diff: test/known_failures_Linux.json ---
@@ -224,5 +224,7 @@
"hs-py3_json_framed-ip",
"java-d_compact_buffered-ip
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1260#discussion_r114529981
--- Diff: test/rs/src/bin/test_server.rs ---
@@ -104,35 +104,43 @@ fn run() -> thrift::Res
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
No problem - I can change it to do that. Should that be in a different
commit/pull-request, or merged in this one?
---
If your project is set up for it, you can reply to this email and have
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
Also, I'm fixing the CentOS issues. I mistakenly assumed that `sh` would be
the same in both ubuntu and Centos. Guess I was wrong...
---
If your project is set up for it, you can reply
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1260
@jeking3 I've updated the travis build with rust support, but I think I'm
running into timeouts because now the builds take too long :/ Any ideas what I
should do?
---
If your project is set
GitHub user allengeorge opened a pull request:
https://github.com/apache/thrift/pull/1260
THRIFT-4186 Add travis build for Rust
Client: rs
* Install Rust in ubuntu/centos docker images
* Fix type bounds on TMultiplexedProcessor
* Add multiplexing support to cross
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1255
@jeking3 That's a very fair point - I didn't realize the cross-tests
weren't running. I try to be disciplined in my testing and ensure nothing
breaks, but I'm only human; automating things
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1255
Updated the PR description with an example. This PR now contains the final
state of all types, etc and should make the runtime code a lot cleaner and
easier to use!
---
If your project is set
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1255
@jeking3 @Jens-G Any chance this could be merged in?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
GitHub user allengeorge opened a pull request:
https://github.com/apache/thrift/pull/1255
THRIFT-4176: Implement threaded server for Rust
Client: rs
* Separate TTransport into two constructs: TIoChannel and TTransport
* Make TIoChannel and TTransport Send-able types
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1187
Yes please - that would be the best course of action.
---
If your project is set up for it, you can
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1246
@jeking3 I did, though unfortunately the communication was all on [the
original Rust Thrift](https://issues.apache.org/jira/browse/THRIFT-2945). I
suggested that Tony - the submitter - split up
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1246
Hmm. Don't know who should review this, considering that I'm the one who
implemented Rust support. But, FWIW, ran the standard cross-tests, checked
clippy output, etc.
---
If your project
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1226
Note that I've tested it against master, and all the rust tests and
cross-tests pass.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1226
@Jens-G This looks good to me. Can it be merged?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1226
Will take a look today.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
Yay! Build passed :) Any thoughts on whether this is OK to merge?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
Fixed makefile errors with `make distdir`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
More updates:
- Minor cleanups to Rust code generators
- Confirmed that all Rust code does not trigger any clippy lint warnings
---
If your project is set up for it, you can reply
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
@anatol Code has been updated with the following changes:
- All usage of `try!` converted to `?`
- Fix all but two clippy lint warnings (I thought the existing code was
clearer
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
@anatol Thanks for the clippy output! I'll look through all of the items
and fix them. I'd no idea that `new()` without `Default` was considered a code
smell. Interesting.
---
If your project
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r97211560
--- Diff: lib/rs/src/errors.rs ---
@@ -0,0 +1,678 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r97211074
--- Diff: lib/rs/src/errors.rs ---
@@ -0,0 +1,678 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r97210903
--- Diff: lib/rs/src/errors.rs ---
@@ -0,0 +1,678 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
Updated:
- Auto-generate constructors for all public structs
- Auto-generate `Default` impl for structs with all-optional fields
- Add constructors for protocol/transport
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
Any thoughts on this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
I've now added the tutorial.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
@Jens-G Sorry to bother you, but all the Appveyor builds seem stalled. It
appears to be holding up the Travis fix #1158 that was approved yesterday as
well :/
---
If your project is set up
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
@anatol No worries! I was glad to do it :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
@anatol My apologies for the earlier spam. FWIW, the latest version of the
PR has the following:
- The "thrift" crate name (I do not use "rift" anywhere)
-
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r96126537
--- Diff: .gitignore ---
@@ -206,6 +206,7 @@ project.lock.json
/lib/delphi/test/typeregistry/*.local
/lib/delphi/test/typeregistry/*.dcu
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r96126534
--- Diff: compiler/cpp/src/thrift/generate/t_rs_generator.cc ---
@@ -2722,7 +2756,30 @@ string t_rs_generator::rust_namespace(t_service*
tservice
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r96126542
--- Diff: .gitignore ---
@@ -281,6 +293,7 @@ project.lock.json
/test/log/
/test/test.log
/test/erl/.generated
+/test/erl/.rebar
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r96126529
--- Diff: lib/rs/Cargo.toml ---
@@ -0,0 +1,17 @@
+[package]
+name = "rift"
--- End diff --
@anatol I've previous
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
[puzzled] I removed the reserved prefix and added an underscore suffix as
you requested. Perhaps I didn't commit that. Let me check. Sorry about that!
And, BTW - I would be very happy
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
Sorry @anatol - I thought I'd changed the package name everywhere! I must
have missed that. Let me fix it ASAP. Sorry!
On Sat, Jan 14, 2017 at 3:20 PM -0500
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
Hi guys - any comments on this PR? I think I've addressed all review
comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
@Jens-G Could you add `lib/rs/README.md` to the list of files for which a
version has to be updated? I will also look and see if I can automate this
somehow.
---
If your project is set up
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
I've now updated the PR with `TMultiplexedProcessor`, which was the last
functional piece missing. I'm now fleshing out the remaining tests as well as
documentation and examples.
---
If your
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
@Jens-G Do you know why the Debian steps in Travis are failing with:
```
The command "docker run --net=host -e BUILD_LIBS="$BUILD_LIBS" $BUILD_ENV
-v $(pwd):/thrift
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
As for crate ownership, I'm happy to publish (and thereby reserve
`thrift`), and then transfer ownership to whoever will be in charge of pushing
published crates.
The relevant
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
OK. I believe I've addressed all outstanding comments.
@anatol:
* Reserved type names now have a `_` suffix. I've confirmed that this does
not trip warnings for either snake
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
OK. Will do.
@anatol Do you know if it's possible for someone on the thrift team to
"reserve" the thrift crate on crates.io? I originally named the library `rift`
because I d
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
Oh. Interesting. I forgot that Debian doesn't have a Rust package (yet)
though it's being worked on IIRC.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
I'm continuing to add tests and update documentation, and I think I've
addressed all the initial comments on this PR. Any further thoughts?
---
If your project is set up for it, you can reply
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
Also, I think the Appveyor build breakage is unrelated?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
Thanks @markerickson-wf - I also saw the commit improving "make clean".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as wel
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
@jsgf For structs it's more involved than it appears - perhaps because of
the way I've chosen to represent them. Here are some factors that cause issues:
1. I represent `required
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
@markerickson-wf Thanks Mark. Here's the error I get when I run `make
precross`:
```
Resolving dependencies...
The pubspec for thrift 0.10.0 from path has version 1.0.0-dev
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r94576295
--- Diff: compiler/cpp/src/thrift/generate/t_rs_generator.cc ---
@@ -0,0 +1,2846 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r94356759
--- Diff: lib/rs/Cargo.toml ---
@@ -0,0 +1,17 @@
+[package]
+name = "rift"
+description = "Rust Thrift library"
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r94354976
--- Diff: lib/rs/Cargo.toml ---
@@ -0,0 +1,17 @@
+[package]
+name = "rift"
+description = "Rust Thrift library"
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r94354857
--- Diff: test/ThriftTest.thrift ---
@@ -67,7 +67,7 @@ typedef i64 UserId
struct Bonk
{
1: string message,
- 2: i32 type
+ 2
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r94331438
--- Diff: lib/rs/Cargo.toml ---
@@ -0,0 +1,17 @@
+[package]
+name = "rift"
+description = "Rust Thrift library"
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r94331337
--- Diff: test/csharp/TestServer.cs ---
@@ -273,39 +273,24 @@ public long testTypedef(long thing)
return mapmap
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1147
I thought that there was no agreed-upon rust codestyle... At least, I think
rustfmt's defaults are winding their way through an RFC process right now
no?
Terminal Musings: http
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r94287266
--- Diff: test/csharp/TestServer.cs ---
@@ -273,39 +273,24 @@ public long testTypedef(long thing)
return mapmap
Github user allengeorge commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1147#discussion_r94287260
--- Diff: test/ThriftTest.thrift ---
@@ -115,14 +115,6 @@ struct CrazyNesting {
4: binary binary_field
}
-union SomeUnion
GitHub user allengeorge opened a pull request:
https://github.com/apache/thrift/pull/1147
THRIFT-2945 Add Rust support
This is a PR to add Rust support to Thrift. It is based on today's master,
and I've verified that Rust server/client successfully communicates with all
cross
Github user allengeorge closed the pull request at:
https://github.com/apache/thrift/pull/1146
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1146
I'm closing and trying again to see if the PR is more tractable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
1 - 100 of 104 matches
Mail list logo
