You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2020/04/03 14:58:50 UTC

[GitHub] [celix] Oipo opened a new pull request #185: Add custom serialization service for custom delete functions in RSA

Oipo opened a new pull request #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185
 
 
   * Refactor rsa activators to celix variant
   * prevent c++ dm making copies of strings all the time
   * Add custom serialization service for custom delete functions in RSA
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] abroekhuis commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-609612734
 
 
   > > I see many small fixed besides the RSA changes. Can you take them out and create a separate PR for it? This clouds the PR for reviewing.
   > 
   > Definitely something I'll improve next PR. For now, I'm only easily able to move the DependencyManager interface changes to a separate PR.
   
   I understand, that would already help. Thanks!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] pnoltes commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#discussion_r404881893
 
 

 ##########
 File path: bundles/remote_services/remote_services_api/include/remote_custom_serialization_service.h
 ##########
 @@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef CELIX_REMOTE_CUSTOM_SERIALIZATION_SERVICE_H
 
 Review comment:
   @Oipo & @abroekhuis  WDYT?
   
   Note that this is for now too much work for this PR. Maybe support the above mentioned rsa_custom_serialization_service and create a ticker for the pubsub?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-610801072
 
 
   +1 to supporting fqn+version. However, I can still think of a use case for also supporting targeting service ids. Suppose a user has a remoted service X with serialization json and twenty java applications that communicate with X. User wants to move to serialization msgpack. Not wanting to upgrade all services communicating with service X at once, the user simply spins up two instances with a different filter (e.g. (service.serialization=json) and (service.serialization=msgpack)) so that upgrading can ben done per java application, instead of having to do twenty at the same time.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] pnoltes commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#discussion_r404871825
 
 

 ##########
 File path: bundles/remote_services/remote_services_api/include/remote_custom_serialization_service.h
 ##########
 @@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef CELIX_REMOTE_CUSTOM_SERIALIZATION_SERVICE_H
 
 Review comment:
   Yeah, I starting to think that we should move away from descriptors and work our way to custom serialization services. Especially because I see more and more combination of C and C++ and then custom serialization would create more freedom how to handle this.
   
   Custom serialization services should not target an individual service (by id), but a message or service by fully qualified name and an optional version.
   
   I think we need to split the serialization for services and message. For service we need a (de)serialisation for the arguments and the return, for message only one set of de/serialization is needed. 
   
   ```C
   //registered with properties targeted.service.fqn, targeted.service.version (optional) 
   //mutiple registration are possible -> order by service ranking
   typedef struct rsa_custom_serialization_service {
        void* handle;
   
       const char* serviceFqn;
       const char* serviceVersion;
   
        int (*serializeArguments)(void *handle, void *args[] /*NULL terminated*/, char **out);
        int (*deserializeArguments)(void *handle, char *data, void **args[] /*output, NULL terminated*/);
        int (*freeArguments)((void* handle, void *args[] /*NULL terminated*/);
   
        int (*serializeReturn)(void *handle, void *returnType, char **out);
        int (*deserializeReturn)(void *handle, char *data, void **out);
        int (*freeReturn)(void* handle, void *obj);
    } rsa_custom_serialization_service_t;
   
   //registered with properties targeted.msg.fqn, targeted.msg.version (optional) and targeted.msg.id (??)
   //mutiple registration are possible -> order by service ranking
   typedef struct pubsub_custom_msg_serializer {
       void* handle;
   
       unsigned int msgId;
       const char* msgFqn;
       const char* msgVersion;
   
       celix_status_t (*serialize)(void* handle, const void* input, void** out, size_t* outLen);
       celix_status_t (*deserialize)(void* handle, const void* input, size_t inputLen, void** out); //note inputLen can be 0 if predefined size is not needed
       void (*freeMsg)(void* handle, void* msg);
   
   } pubsub_custom_msg_serializer_t; //note almost the same as the current pubsub_msg_serializer_t
    
   ```
   
   Descriptors can still be supported, but then as an extender pattern. So a json (descriptor) serializer would monitor the bundles and register a service for every descriptor found. If there is already a serializer for that descriptor, skip it. 
   
   PubSub admin will need to ensure that the custom serialization services are map by msg id instead of names.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] abroekhuis commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#discussion_r403870286
 
 

 ##########
 File path: bundles/remote_services/topology_manager/src/activator.c
 ##########
 @@ -46,98 +47,42 @@
 struct activator {
 	celix_bundle_context_t *context;
 
-	topology_manager_pt manager;
+	topology_manager_t *manager;
 
-	service_tracker_t *endpointListenerTracker;
-	service_tracker_t *remoteServiceAdminTracker;
-	service_tracker_t *exportedServicesTracker;
+    celix_service_tracker_t *endpointListenerTracker;
 
 Review comment:
   Spacing is not right

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] abroekhuis commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-610806752
 
 
   > +1 to supporting fqn+version. However, I can still think of a use case for also supporting targeting service ids. Suppose a user has a remoted service X with serialization json and twenty java applications that communicate with X. User wants to move to serialization msgpack. Not wanting to upgrade all services communicating with service X at once, the user simply spins up two instances with a different filter (e.g. (service.serialization=json) and (service.serialization=msgpack)) so that upgrading can ben done per java application, instead of having to do twenty at the same time.
   
   If a list of message types or something is used in the service properties, both could support the same types, but with service ranking preference to one of the other could be given.
   So in your example:
   JSON has a service property: message.types={ "a", "b", "c"} with a ranking of 10
   MSGPack has a service property: message.types={"a", "c"} with a ranking of 20
   In this case the RSA would still pick up both serializers, and starts at the highest ranking to find a handler for a message, moving down the list.
   So for msg "a" it would use MSGPack, while for msg "b" it would use JSON.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] pnoltes commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#discussion_r404877618
 
 

 ##########
 File path: bundles/remote_services/remote_services_api/include/remote_custom_serialization_service.h
 ##########
 @@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef CELIX_REMOTE_CUSTOM_SERIALIZATION_SERVICE_H
+#define CELIX_REMOTE_CUSTOM_SERIALIZATION_SERVICE_H
+
+#define RSA_CUSTOM_SERIALIZATION_SERVICE_NAME "rsa.custom.serialization"
+#define RSA_SERVICE_TARGETED_ID "targeted.service.id"
 
 Review comment:
   I do not like a target service id. This means that the service needs to be registered before the custom serialisation service and I would prefer it the other way around. 
   
   Or we should make the custom serialization service mandatory. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] abroekhuis commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-609632407
 
 
   > > > Clang on macOS fails because of a bug in clang: https://bugs.llvm.org/show_bug.cgi?id=21629
   > > > Suggest adding -Wno-missing-braces.
   > > 
   > > 
   > > Why is it an issue suddenly? What line of code triggers it?
   > > From the logs:
   > 
   > ```
   > 2020-04-06T07:35:34.3719830Z /Users/runner/runners/2.168.0/work/celix/celix/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c:275:46: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
   > 2020-04-06T07:35:34.3820920Z     celix_service_tracking_options_t opts = {0};
   > ```
   > 
   > Using {0} is a standard compliant way to initialize all members of a struct to 0.
   
   OK, I see, instead of {0}, can you use CELIX_EMPTY_SERVICE_TRACKING_OPTIONS, as is done in other places. That way we can leave the build options the same.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] abroekhuis commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-608644818
 
 
   > Clang on macOS fails because of a bug in clang: https://bugs.llvm.org/show_bug.cgi?id=21629
   > 
   > Suggest adding -Wno-missing-braces.
   
   Why is it an issue suddenly? What line of code triggers it?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] abroekhuis commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#discussion_r405319837
 
 

 ##########
 File path: bundles/remote_services/remote_services_api/include/remote_custom_serialization_service.h
 ##########
 @@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef CELIX_REMOTE_CUSTOM_SERIALIZATION_SERVICE_H
 
 Review comment:
   How does this relate to the existing serializers already in pubsub? Most is already in place there, right?
   Other than that, I think this sounds like a good approach. If we can solve the RSA part in this PR, that would be great!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-609621872
 
 
   > > > I see many small fixed besides the RSA changes. Can you take them out and create a separate PR for it? This clouds the PR for reviewing.
   > > 
   > > 
   > > Definitely something I'll improve next PR. For now, I'm only easily able to move the DependencyManager interface changes to a separate PR.
   > 
   > I understand, that would already help. Thanks!
   
   All the changes to it have been reverted.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] abroekhuis commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#discussion_r403870080
 
 

 ##########
 File path: bundles/remote_services/remote_services_api/include/remote_custom_serialization_service.h
 ##########
 @@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef CELIX_REMOTE_CUSTOM_SERIALIZATION_SERVICE_H
 
 Review comment:
   This header has a TODO to add serialize/deserialize to it as well, does that make sense?
   @pnoltes I think it does, just want to make sure we don't overlook something. Is it, for example, possible to not use a serialization solution/service?
   Also, currently serialization is not in a service at all, if we do this, I think it makes sense to move it asap as well. Current situation could just be a wrapper for the used json_rpc I think.
   Also, if we do it like this for the RSA, I think a similar solution is needed for the PSA, wdyt?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-610817825
 
 
   > > +1 to supporting fqn+version. However, I can still think of a use case for also supporting targeting service ids. Suppose a user has a remoted service X with serialization json and twenty java applications that communicate with X. User wants to move to serialization msgpack. Not wanting to upgrade all services communicating with service X at once, the user simply spins up two instances with a different filter (e.g. (service.serialization=json) and (service.serialization=msgpack)) so that upgrading can ben done per java application, instead of having to do twenty at the same time.
   > 
   > If a list of message types or something is used in the service properties, both could support the same types, but with service ranking preference to one of the other could be given.
   > So in your example:
   > JSON has a service property: message.types={ "a", "b", "c"} with a ranking of 10
   > MSGPack has a service property: message.types={"a", "c"} with a ranking of 20
   > In this case the RSA would still pick up both serializers, and starts at the highest ranking to find a handler for a message, moving down the list.
   > So for msg "a" it would use MSGPack, while for msg "b" it would use JSON.
   
   Oh, perfect! Then nevermind my comment :)
   
   IMO, this PR can be (ab)used for implementing the suggestion of Pepijn; people will most likely wait to use the service until after the serde functions have been implemented, as that would prevent double work.
   
   Only downside is if it fits in the scope of our sprint or not :)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-608503402
 
 
   Clang on macOS fails because of a bug in clang: https://bugs.llvm.org/show_bug.cgi?id=21629
   
   Suggest adding -Wno-missing-braces.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-609612076
 
 
   > I see many small fixed besides the RSA changes. Can you take them out and create a separate PR for it? This clouds the PR for reviewing.
   
   Definitely something I'll improve next PR. For now, I'm only easily able to move the DependencyManager interface changes to a separate PR.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
Oipo commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-609628486
 
 
   > > Clang on macOS fails because of a bug in clang: https://bugs.llvm.org/show_bug.cgi?id=21629
   > > Suggest adding -Wno-missing-braces.
   > 
   > Why is it an issue suddenly? What line of code triggers it?
   From the logs:
   ```
   2020-04-06T07:35:34.3719830Z /Users/runner/runners/2.168.0/work/celix/celix/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c:275:46: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
   2020-04-06T07:35:34.3820920Z     celix_service_tracking_options_t opts = {0};
   ```
   
   Using {0} is a standard compliant way to initialize all members of a struct to 0.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] abroekhuis commented on issue #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on issue #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#issuecomment-608644492
 
 
   I see many small fixed besides the RSA changes. Can you take them out and create a separate PR for it? This clouds the PR for reviewing.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [celix] abroekhuis commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on a change in pull request #185: Add custom serialization service for custom delete functions in RSA
URL: https://github.com/apache/celix/pull/185#discussion_r405322454
 
 

 ##########
 File path: bundles/remote_services/remote_services_api/include/remote_custom_serialization_service.h
 ##########
 @@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef CELIX_REMOTE_CUSTOM_SERIALIZATION_SERVICE_H
+#define CELIX_REMOTE_CUSTOM_SERIALIZATION_SERVICE_H
+
+#define RSA_CUSTOM_SERIALIZATION_SERVICE_NAME "rsa.custom.serialization"
+#define RSA_SERVICE_TARGETED_ID "targeted.service.id"
 
 Review comment:
   I also think a service.id is wrong, because now the serialization always needs to figure out the service id of the target. Using a service property (list of types/messages supported by this bundle) would make more sense.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services