refactor: Re-design streaming services.

[prmukherj]

Context

In previous PRs all the grpc services were properly segregated so that the low level grpc code remains in _grpc_services and the high level python layer in services. Now the streaming services also requires similar redesign. Moreover, there were instances of server_supports_v1 leaking in the python codebase.

Change Summary

1. Streaming services redesigned in a similar manner so that the grpc related code remains in the _grpc_services and the high level python code in streaming_services
2. There are no more leaking instances of server_supports_v1 in the PyFluent codebase.
3. The proto version handling, etc. have all been taken care of in the low level factory.
4. The PyFluent session code is much cleaner now with no unnecessary imports and having the services directly available via fluent_connection.service_factory.

Rationale

This was done to provide a clean segregation to the PyFluent users, so that in future other services can be plugged in with minimal effort.

Impact

No impact to users.
All tests running fine.

Services covered in this refactoring:

1. Datamodel Event Streaming
2. Datamodel Streaming
3. Events Streaming
4. Field Data Streaming
5. Monitor Streaming
6. Transcript Streaming

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[seanpearsonuk]

@prmukherj Please make the description detailed so that reviewers know where to look and for what.

[prmukherj]

@prmukherj Please make the description detailed so that reviewers know where to look and for what.

On it.

[seanpearsonuk]

General comments

1. GRPCServiceFactory and ServiceFactory both use internal conditionals where separate factories would make more sense. Use abstract factory pattern with some methods raising NotImplementedError in derived classes.
2. GRPCServiceFactory stores unused attributes.
3. error_state is plumbed all the way through into services where separate interceptors are eventually created - multiple times. intercept_channel should be created once in the factory and passed to each service. Error state is an unwanted service dependency.
4. Proto version attribute is too weakly typed.

[seanpearsonuk]

Initial comments from read through of _grpc_services.__init__.

[prmukherj]

General comments

1. GRPCServiceFactory and ServiceFactory both use internal conditionals where separate factories would make more sense. Use abstract factory pattern with some methods raising NotImplementedError in derived classes.
2. GRPCServiceFactory stores unused attributes.
3. error_state is plumbed all the way through into services where separate interceptors are eventually created - multiple times. intercept_channel should be created once in the factory and passed to each service. Error state is an unwanted service dependency.
4. Proto version attribute is too weakly typed.

@seanpearsonuk, I have implemented all your suggestions except 3. Why I had initially avoided it is because some services like batch ops and solution variables does not require the full stack of error interceptors, whereas some like events and transcript does not require any interceptors and initialize with just the raw channel. There are 2 options here to do this without tampering with the design much, one is if we pass these contexts from factory, which is again not a good idea. Other is if we somehow have these contexts in the service classes as class variables and can be queried and based on that we pass the required type of channel to it. The easiest option was to pass the intercept channel with the full set of interceptors defined for every service but I wanted your feedback before doing that as it will affect performance. Even if a service does not use the ErrorStateInterceptor, TracingInterceptor, or BatchInterceptor, gRPC will still force every single remote procedure call (RPC) through those interceptors.

Please provide your views on this one. Thanks

[prmukherj]

Nightly test runs: https://github.com/ansys/pyfluent/actions/runs/29889780984

[seanpearsonuk]

General comments

1. GRPCServiceFactory and ServiceFactory both use internal conditionals where separate factories would make more sense. Use abstract factory pattern with some methods raising NotImplementedError in derived classes.
2. GRPCServiceFactory stores unused attributes.
3. error_state is plumbed all the way through into services where separate interceptors are eventually created - multiple times. intercept_channel should be created once in the factory and passed to each service. Error state is an unwanted service dependency.
4. Proto version attribute is too weakly typed.

@seanpearsonuk, I have implemented all your suggestions except 3. Why I had initially avoided it is because some services like batch ops and solution variables does not require the full stack of error interceptors, whereas some like events and transcript does not require any interceptors and initialize with just the raw channel. There are 2 options here to do this without tampering with the design much, one is if we pass these contexts from factory, which is again not a good idea. Other is if we somehow have these contexts in the service classes as class variables and can be queried and based on that we pass the required type of channel to it. The easiest option was to pass the intercept channel with the full set of interceptors defined for every service but I wanted your feedback before doing that as it will affect performance. Even if a service does not use the ErrorStateInterceptor, TracingInterceptor, or BatchInterceptor, gRPC will still force every single remote procedure call (RPC) through those interceptors.

Please provide your views on this one. Thanks

My proposal doesn't imply the situation you describe. It also solves more than one design issue.

[prmukherj]

General comments

1. GRPCServiceFactory and ServiceFactory both use internal conditionals where separate factories would make more sense. Use abstract factory pattern with some methods raising NotImplementedError in derived classes.
2. GRPCServiceFactory stores unused attributes.
3. error_state is plumbed all the way through into services where separate interceptors are eventually created - multiple times. intercept_channel should be created once in the factory and passed to each service. Error state is an unwanted service dependency.
4. Proto version attribute is too weakly typed.

@seanpearsonuk, I have implemented all your suggestions except 3. Why I had initially avoided it is because some services like batch ops and solution variables does not require the full stack of error interceptors, whereas some like events and transcript does not require any interceptors and initialize with just the raw channel. There are 2 options here to do this without tampering with the design much, one is if we pass these contexts from factory, which is again not a good idea. Other is if we somehow have these contexts in the service classes as class variables and can be queried and based on that we pass the required type of channel to it. The easiest option was to pass the intercept channel with the full set of interceptors defined for every service but I wanted your feedback before doing that as it will affect performance. Even if a service does not use the ErrorStateInterceptor, TracingInterceptor, or BatchInterceptor, gRPC will still force every single remote procedure call (RPC) through those interceptors.
Please provide your views on this one. Thanks

My proposal doesn't imply the situation you describe. It also solves more than one design issue.

Something like this is suggested right?

[seanpearsonuk]

General comments

1. GRPCServiceFactory and ServiceFactory both use internal conditionals where separate factories would make more sense. Use abstract factory pattern with some methods raising NotImplementedError in derived classes.
2. GRPCServiceFactory stores unused attributes.
3. error_state is plumbed all the way through into services where separate interceptors are eventually created - multiple times. intercept_channel should be created once in the factory and passed to each service. Error state is an unwanted service dependency.
4. Proto version attribute is too weakly typed.

@seanpearsonuk, I have implemented all your suggestions except 3. Why I had initially avoided it is because some services like batch ops and solution variables does not require the full stack of error interceptors, whereas some like events and transcript does not require any interceptors and initialize with just the raw channel. There are 2 options here to do this without tampering with the design much, one is if we pass these contexts from factory, which is again not a good idea. Other is if we somehow have these contexts in the service classes as class variables and can be queried and based on that we pass the required type of channel to it. The easiest option was to pass the intercept channel with the full set of interceptors defined for every service but I wanted your feedback before doing that as it will affect performance. Even if a service does not use the ErrorStateInterceptor, TracingInterceptor, or BatchInterceptor, gRPC will still force every single remote procedure call (RPC) through those interceptors.
Please provide your views on this one. Thanks

My proposal doesn't imply the situation you describe. It also solves more than one design issue.

Something like this is suggested right?

Avoid agglomerating the various service dependencies. Construct each service by passing what the service needs which is also what it should request. Both of these points are also general rules.