Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Statestore apis to convenient layer #74

Closed
4 tasks
youngbupark opened this issue Jun 22, 2020 · 16 comments · Fixed by #100 or #103
Closed
4 tasks

Add Statestore apis to convenient layer #74

youngbupark opened this issue Jun 22, 2020 · 16 comments · Fixed by #100 or #103
Assignees
Labels
P1 size/M 2 weeks of work

Comments

@youngbupark
Copy link
Contributor

Describe the proposal

Add Statestore apis to convenient layer.

Parent epic : #67

  • Implementation
  • Unit-tests
  • Documentation in code
  • update examples
@youngbupark youngbupark added the P1 label Jun 22, 2020
@youngbupark
Copy link
Contributor Author

@chinzhiweiblank Please leave any comment here. I will assign this issue to you :)

@youngbupark
Copy link
Contributor Author

@chinzhiweiblank please review this pr - #78

This is the initial pr for invoke_service. I am trying to merge this by monday pst.
You can add state api to Dapr class and need to define Response class for state api inheriting from DaprResponse. Tests needs be added.

@youngbupark
Copy link
Contributor Author

@chinzhiweiblank Please wait until the pr #78 is merged. #78 includes lots of initial code which can be used for the other rest of apis. please review the #78 pr first and put your change for statestore on hold until this pr is merged.

@youngbupark
Copy link
Contributor Author

@chinzhiweiblank We finally merged #78 PR. Please continue to work on statestore apis.

@chinzhiweiblank
Copy link
Contributor

@youngbupark no problem

@youngbupark
Copy link
Contributor Author

@youngbupark no problem

Thanks! Please create the small pr with unittest. For example, we will need implement 3 methods for state store. Then please create 3 PRs for each method. Also, Dapr class has been renamed to DaprClient.

@youngbupark youngbupark added the size/M 2 weeks of work label Jul 20, 2020
@youngbupark
Copy link
Contributor Author

@chinzhiweiblank any updates ?

@chinzhiweiblank
Copy link
Contributor

@chinzhiweiblank any updates ?

Still not done yet. I will open the PR by this week.

@youngbupark
Copy link
Contributor Author

@chinzhiweiblank any updates ?

Still not done yet. I will open the PR by this week.

Cool. Please file the small PRs for each method.

@youngbupark
Copy link
Contributor Author

@amanbha @orizohar This issue has been worked by external contributor. so I will keep this in 0.10.0.

@chinzhiweiblank
Copy link
Contributor

Hey @youngbupark I had these error messages while executing the grpc tests in test_dapr_grpc_client.py. Any idea on how to resolve them?

ERROR: test_publish_event (clients.test_dapr_grpc_client.DaprGrpcClientTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/zhiwei/Desktop/python-sdk/tests/clients/test_dapr_grpc_client.py", line 139, in test_publish_event
    data=b'haha',
  File "/home/zhiwei/Desktop/python-sdk/dapr/clients/grpc/client.py", line 280, in publish_event
    response, call = self._stub.PublishEvent.with_call(req, metadata=metadata)
  File "/home/zhiwei/.local/share/virtualenvs/python-sdk-5KBjW-bk/lib/python3.7/site-packages/grpc/_channel.py", line 835, in with_call
    return _end_unary_response_blocking(state, call, True, None)
  File "/home/zhiwei/.local/share/virtualenvs/python-sdk-5KBjW-bk/lib/python3.7/site-packages/grpc/_channel.py", line 726, in _end_unary_response_blocking
    raise _InactiveRpcError(state)
grpc._channel._InactiveRpcError: <_InactiveRpcError of RPC that terminated with:
        status = StatusCode.UNAVAILABLE
        details = "Trying to connect an http1.x server"
        debug_error_string = "{"created":"@1595609345.600031171","description":"Error received from peer ipv6:[::1]:8080","file":"src/core/lib/surface/call.cc","file_line":1056,"grpc_message":"Trying to connect an http1.x server","grpc_status":14}"
>

@youngbupark
Copy link
Contributor Author

Hey @youngbupark I had these error messages while executing the grpc tests in test_dapr_grpc_client.py. Any idea on how to resolve them?

ERROR: test_publish_event (clients.test_dapr_grpc_client.DaprGrpcClientTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/zhiwei/Desktop/python-sdk/tests/clients/test_dapr_grpc_client.py", line 139, in test_publish_event
    data=b'haha',
  File "/home/zhiwei/Desktop/python-sdk/dapr/clients/grpc/client.py", line 280, in publish_event
    response, call = self._stub.PublishEvent.with_call(req, metadata=metadata)
  File "/home/zhiwei/.local/share/virtualenvs/python-sdk-5KBjW-bk/lib/python3.7/site-packages/grpc/_channel.py", line 835, in with_call
    return _end_unary_response_blocking(state, call, True, None)
  File "/home/zhiwei/.local/share/virtualenvs/python-sdk-5KBjW-bk/lib/python3.7/site-packages/grpc/_channel.py", line 726, in _end_unary_response_blocking
    raise _InactiveRpcError(state)
grpc._channel._InactiveRpcError: <_InactiveRpcError of RPC that terminated with:
        status = StatusCode.UNAVAILABLE
        details = "Trying to connect an http1.x server"
        debug_error_string = "{"created":"@1595609345.600031171","description":"Error received from peer ipv6:[::1]:8080","file":"src/core/lib/surface/call.cc","file_line":1056,"grpc_message":"Trying to connect an http1.x server","grpc_status":14}"
>

it is working on my machine and github actions. this unit-test starts grpc test server according to the error message, your os firewall might block the test server port

@chinzhiweiblank
Copy link
Contributor

chinzhiweiblank commented Jul 26, 2020

@youngbupark I'm almost getting the implementation to work. Just needed to clarify something for save-states

        """Saves key-value pairs to a statestore
        The example saves states to a statestore:
            from dapr import DaprClient
            with DaprClient() as d:
                resp = d.save_state(
                    store_name='state_store'
                    states=[{'key': 'key1', 'value': 'value1'}],
                )
        Args:
            store_name (str): the state store name to save to
            states (List[dict]): the key-value pairs to be saved
        Returns:
            None
        """

Should the states be a list of dictionaries or bytes or a JSON string? I am converting them into common_v1.StateItem.

@youngbupark
Copy link
Contributor Author

youngbupark commented Jul 26, 2020

@youngbupark I'm almost getting the implementation to work. Just needed to clarify something for save-states

        """Saves key-value pairs to a statestore
        The example saves states to a statestore:
            from dapr import DaprClient
            with DaprClient() as d:
                resp = d.save_state(
                    store_name='state_store'
                    states=[{'key': 'key1', 'value': 'value1'}],
                )
        Args:
            store_name (str): the state store name to save to
            states (List[dict]): the key-value pairs to be saved
        Returns:
            None
        """

Should the states be a list of dictionaries or bytes or a JSON string? I am converting them into common_v1.StateItem.

Good question. as I initially mentioned, please refer to this java-sdk implementation.

https://github.com/dapr/java-sdk/blob/8e19ba68c9094ebbc2eeff36343d99b318f8bb4a/sdk/src/main/java/io/dapr/client/DaprClientGrpc.java#L495-L498

We will support single state save/get/delete not multiple ones. each state option should be able to set the concurrency/consistency in state options. please refer java-sdk implementation.

https://github.com/dapr/dapr/blob/19e6d3256f62a46c0a3df1b612b7081336458e9a/dapr/proto/common/v1/common.proto#L84-L100

@chinzhiweiblank thanks for your contribution.

@youngbupark
Copy link
Contributor Author

@chinzhiweiblank when it comes to json serialization. The api layer will get only bytes and str and let users to serialize and deserialize object for the current implementation.

@youngbupark
Copy link
Contributor Author

@chinzhiweiblank if you create a draft pr, then we can work together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 size/M 2 weeks of work
Projects
None yet
2 participants