[jira] [Commented] (THRIFT-4639) Sequence numbering for multiplexed protocol broken
[ https://issues.apache.org/jira/browse/THRIFT-4639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16630923#comment-16630923 ] ASF GitHub Bot commented on THRIFT-4639: bforbis edited a comment on issue #1597: THRIFT-4639: Use correct sequence number for multiplexed protocol URL: https://github.com/apache/thrift/pull/1597#issuecomment-425210116 To run the node tests, follow the instructions in the [docker README](https://github.com/apache/thrift/tree/master/build/docker) for creating your test image. You can then either run the full test suite with `autotools.sh`, or if you want to just run the nodeJS tests you should be able to run `make check` within the node lib directory. Alternatively, if you understand how grunt files work you can just use that directly. I've got an open PR in #1584 which is doing a sizeable refactor, so our changes may conflict This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Sequence numbering for multiplexed protocol broken > -- > > Key: THRIFT-4639 > URL: https://issues.apache.org/jira/browse/THRIFT-4639 > Project: Thrift > Issue Type: Bug > Components: Node.js - Library >Affects Versions: 0.11.0 >Reporter: PH Lundblom >Priority: Blocker > > Handling of client sequence numbering for multiplexed protocol is broken. > Current implementation uses client internal variable "seqid" which should be > "_seqid" > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4639) Sequence numbering for multiplexed protocol broken
[ https://issues.apache.org/jira/browse/THRIFT-4639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16630921#comment-16630921 ] ASF GitHub Bot commented on THRIFT-4639: bforbis commented on issue #1597: THRIFT-4639: Use correct sequence number for multiplexed protocol URL: https://github.com/apache/thrift/pull/1597#issuecomment-425210116 To run the node tests, follow the instructions in the [docker README](https://github.com/apache/thrift/tree/master/build/docker) for creating your test image. You can then either run the full test suite with `autotools.sh`, or if you want to just run the nodeJS tests you should be able to run `make check` within the node lib directory. Alternatively, if you understand how grunt files work you can just use that directly. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Sequence numbering for multiplexed protocol broken > -- > > Key: THRIFT-4639 > URL: https://issues.apache.org/jira/browse/THRIFT-4639 > Project: Thrift > Issue Type: Bug > Components: Node.js - Library >Affects Versions: 0.11.0 >Reporter: PH Lundblom >Priority: Blocker > > Handling of client sequence numbering for multiplexed protocol is broken. > Current implementation uses client internal variable "seqid" which should be > "_seqid" > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4541) Use new project system in "lib/csharp" and define supported platforms
[ https://issues.apache.org/jira/browse/THRIFT-4541?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16630898#comment-16630898 ] Christian Weiss commented on THRIFT-4541: - There seem to be some misunderstandings here. Both folders, “csharp” and “netcore”, are written in C#. The main difference in the code is that the “netcore” project uses a lot of async/await code, apart from that it’s pretty much the same. So from a code perspective, IMO the “netcore” folder is like a fork of the “csharp” project. It might be interesting to merge them, but that’s a different/bigger project of course. .NET Core is not a new language, so saying that .NET Core would make C# legacy isn’t possible. What’s being discussed here is the PLATFORM support. (Full .NET framework, .NET Core, Xamarin, Mono, ...) This is all about tooling and how the projects are compiled to get the proper nuget packages. this PR is just about switching to the new project format which makes targeting multiple platforms much easier. > Use new project system in "lib/csharp" and define supported platforms > - > > Key: THRIFT-4541 > URL: https://issues.apache.org/jira/browse/THRIFT-4541 > Project: Thrift > Issue Type: Umbrella > Components: C# - Library >Reporter: Christian Weiss >Priority: Major > > As discussed in THRIFT-4535, it would be great if we could update > "lib/csharp" to use the new "csproj" project system. This will allow us to > target multiple platforms and the new ".NET Standard" with a single project. > It's possible to support pretty much every platform there is with this new > project format (see e.g. > [Newtonsoft.Json|https://github.com/JamesNK/Newtonsoft.Json/blob/master/Src/Newtonsoft.Json/Newtonsoft.Json.csproj]), > however supporting older platforms requires more work as this requires more > #if statements etc. > This means that we have to decide which platforms we want to support! > Targeting ".NET Standard 2.0" would be the easiest option as this version > [covers a much larger API > surface|https://blogs.msdn.microsoft.com/dotnet/2017/08/14/announcing-net-standard-2-0/]. > However, this would also mean that people have to use rather recent versions > of their platforms to use it, as .NET Standard 2.0 requires the following > minimum versions: > * .NET 4.6.1 (adding support for .NET 4.5 is no problem though) > * .NET Core 2.0 > * Mono 5.4 > * Xamarin.iOS 10.14 > * Xamarin.Mac 3.8 > * Xamarin.Android 8.0 > * UWP 10.0.16299 > I will create a PR that targets .NET Standard 2.0 and .NET 4.5 to show what > the new project files would look like. If the approach is OK for you in > general, then I can try to add support for whatever minimum versions you'd > like to support. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (THRIFT-4643) Update links to repository on project website
David Jung created THRIFT-4643: -- Summary: Update links to repository on project website Key: THRIFT-4643 URL: https://issues.apache.org/jira/browse/THRIFT-4643 Project: Thrift Issue Type: Task Components: Website Reporter: David Jung The project website at [https://thrift.apache.org|https://thrift.apache.org/] indicates that the source Git repository is located at [https://git-wip-us.apache.org/repos/asf/thrift.git] and links to it in several placed. However, clicking those links or attempting to clone from them just give a 'not found' error. Please update the links. The GitHub unofficial repository is mentioned once at the bottom, but new developers have no idea what that contains in relation to the official repo. Thanks. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4496) Dealing with language keywords in Thrift (e.g. service method names)
[
https://issues.apache.org/jira/browse/THRIFT-4496?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16630288#comment-16630288
]
ASF GitHub Bot commented on THRIFT-4496:
jeking3 commented on issue #1567: THRIFT-4496: python specific list of keywords
for python generator
URL: https://github.com/apache/thrift/pull/1567#issuecomment-425068711
I believe the net effect is that as long as you generate thrift for all the
possible languages you will be using now and in the future, you will get the
keyword validation you desire, without worrying about keywords from languages
you will never use. I'm going to push the build forward and see if it will
complete, and if not then see if it's just one of those intermittent issues or
something we already solved. There's only one build job outstanding right now.
Then we can merge and move this forward. Sorry for the delay.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Dealing with language keywords in Thrift (e.g. service method names)
>
>
> Key: THRIFT-4496
> URL: https://issues.apache.org/jira/browse/THRIFT-4496
> Project: Thrift
> Issue Type: New Feature
> Components: Compiler (General)
>Reporter: Vera Filippova
>Priority: Minor
>
> Apache Thrift compiler doesn't allow to use keywords in any of supported
> languages as field names. However, there are other compilers, like Scrooge,
> which do allow using some keywords as field identifiers, which leads to
> incompatibility.
> Assume we had a service with 'delete' method, with Java code generated by
> Scrooge. Now we'd like to generate Python code with Apache Thrift, but
> encounter an error because of the 'delete' keyword.
> I understand that using only Apache Thrift compiler, a user will never
> encounter this problem, but I think enabling keywords by request seems
> feasible.
> h1. Proposal
> It's possible to tweak keywords on code generation stage, e.g. use 'delete_'
> as a name of a generated function instead of 'delete', then use the original
> method name for a protocol message: writeMethodBegin('delete').
> This feature could be enabled with an additional flag, e.g. --screen-keywords.
> I have a draft for python generator here [https://github.com/nsrtvwls/thrift]
> The questions are, is this functionality welcome? If yes, would it require to
> have it supported for all languages?
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4496) Dealing with language keywords in Thrift (e.g. service method names)
[
https://issues.apache.org/jira/browse/THRIFT-4496?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16630285#comment-16630285
]
ASF GitHub Bot commented on THRIFT-4496:
jeking3 commented on a change in pull request #1567: THRIFT-4496: python
specific list of keywords for python generator
URL: https://github.com/apache/thrift/pull/1567#discussion_r220898633
##
File path: compiler/cpp/src/thrift/generate/t_generator.h
##
@@ -96,7 +97,33 @@ class t_generator {
return escape_string(constval->get_string());
}
+ /**
+ * Check if all identifiers are valid for the target language
+ */
+ virtual void validate_input() const;
+
protected:
+ virtual std::set lang_keywords() const;
+
+ /**
+ * A list of reserved words that cannot be used as identifiers.
+ */
+ const std::set keywords_;
+
+ virtual void validate_id(const std::string& id) const;
+
+ virtual void validate(t_enum const* en) const;
Review comment:
validate looks like an opportunity to use template specialization, since
half of them do the same thing. Just a thought. :)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Dealing with language keywords in Thrift (e.g. service method names)
>
>
> Key: THRIFT-4496
> URL: https://issues.apache.org/jira/browse/THRIFT-4496
> Project: Thrift
> Issue Type: New Feature
> Components: Compiler (General)
>Reporter: Vera Filippova
>Priority: Minor
>
> Apache Thrift compiler doesn't allow to use keywords in any of supported
> languages as field names. However, there are other compilers, like Scrooge,
> which do allow using some keywords as field identifiers, which leads to
> incompatibility.
> Assume we had a service with 'delete' method, with Java code generated by
> Scrooge. Now we'd like to generate Python code with Apache Thrift, but
> encounter an error because of the 'delete' keyword.
> I understand that using only Apache Thrift compiler, a user will never
> encounter this problem, but I think enabling keywords by request seems
> feasible.
> h1. Proposal
> It's possible to tweak keywords on code generation stage, e.g. use 'delete_'
> as a name of a generated function instead of 'delete', then use the original
> method name for a protocol message: writeMethodBegin('delete').
> This feature could be enabled with an additional flag, e.g. --screen-keywords.
> I have a draft for python generator here [https://github.com/nsrtvwls/thrift]
> The questions are, is this functionality welcome? If yes, would it require to
> have it supported for all languages?
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4639) Sequence numbering for multiplexed protocol broken
[ https://issues.apache.org/jira/browse/THRIFT-4639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16629935#comment-16629935 ] ASF GitHub Bot commented on THRIFT-4639: phlundblom commented on issue #1597: THRIFT-4639: Use correct sequence number for multiplexed protocol URL: https://github.com/apache/thrift/pull/1597#issuecomment-425005337 How does one run the NodeJS tests? Unfortunately I don't find it obvious from the Makefiles and/or the Travis outputs This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Sequence numbering for multiplexed protocol broken > -- > > Key: THRIFT-4639 > URL: https://issues.apache.org/jira/browse/THRIFT-4639 > Project: Thrift > Issue Type: Bug > Components: Node.js - Library >Affects Versions: 0.11.0 >Reporter: PH Lundblom >Priority: Blocker > > Handling of client sequence numbering for multiplexed protocol is broken. > Current implementation uses client internal variable "seqid" which should be > "_seqid" > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
