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

[Python] data subscription pages batch [Replace C# samples] #1865

Merged
merged 4 commits into from
Jul 10, 2024

Conversation

reebhub
Copy link
Contributor

@reebhub reebhub commented Jul 3, 2024

RDoc-2925 Data subscriptions > Creation > How to create data subscription
RDoc-2917 Data subscriptions > Creation > Examples
RDoc-2918 Data subscriptions > Creation > API overview
RDoc-2919 Data subscriptions > Consumption > How to consume
RDoc-2920 Data subscriptions > Consumption > Examples
RDoc-2921 Data subscriptions > Consumption > API overview

@reebhub reebhub requested a review from poissoncorp July 3, 2024 22:47
| Member | Type | Description |
|--------|:-----|-------------|
| **subscription_name** | `str` | Returns the subscription name passed to the constructor. This name will be used by the server side to identify the subscription. |
| **time_to_wait_before_connection_retry** | `DateTime` | Time to wait before reconnecting, in the case of non-aborting failure during the subscription processing.<br>Default: 5 seconds. |
Copy link

@poissoncorp poissoncorp Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **time_to_wait_before_connection_retry** | `DateTime` | Time to wait before reconnecting, in the case of non-aborting failure during the subscription processing.<br>Default: 5 seconds. |
| **time_to_wait_before_connection_retry** | `timedelta` | Time to wait before reconnecting, in the case of non-aborting failure during the subscription processing.<br>Default: 5 seconds. |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

After [generating](../../../client-api/data-subscriptions/consumption/api-overview#subscription-worker-generation)
a subscription worker, the worker doesn't process any documents yet.
Processing starts when the `run` function is called.
The `run` function receives the client-side code as a delegate that will process the retrieved batches:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `run` function receives the client-side code as a delegate that will process the retrieved batches:
The `run` function receives the client-side code as a function that will process the retrieved batches:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

node is currently responsible for data subscriptions.
{NOTE/}

| `SubscriptionBatch[_T].item` Member | Type | Description |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `SubscriptionBatch[_T].item` Member | Type | Description |
| `SubscriptionBatch[_T].Item` Member | Type | Description |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


| Method | Return Type | Description |
|--------|:-----|-------------|
| `dispose` | `void` | Aborts subscription worker operation ungracefully by waiting for the task returned by the `run` function to finish running. |
Copy link

@poissoncorp poissoncorp Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `dispose` | `void` | Aborts subscription worker operation ungracefully by waiting for the task returned by the `run` function to finish running. |
| `close(bool wait_for_subscription_task = True)` | `void` | Aborts subscription worker operation ungracefully by waiting for the task returned by the `run` function to finish running. |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

| Method | Return Type | Description |
|--------|:-----|-------------|
| `dispose` | `void` | Aborts subscription worker operation ungracefully by waiting for the task returned by the `run` function to finish running. |
| `dispose(bool waitForSubscriptionTask)` | `void` | Aborts the subscription worker, but allows deciding whether to wait for the `run` function task or not. |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `dispose(bool waitForSubscriptionTask)` | `void` | Aborts the subscription worker, but allows deciding whether to wait for the `run` function task or not. |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaced with close

|--------|:-----|-------------|
| `dispose` | `void` | Aborts subscription worker operation ungracefully by waiting for the task returned by the `run` function to finish running. |
| `dispose(bool waitForSubscriptionTask)` | `void` | Aborts the subscription worker, but allows deciding whether to wait for the `run` function task or not. |
| `run` | `Task` | Starts the subscription worker work of processing batches, receiving the batch processing delegates (see [above](../../../client-api/data-subscriptions/consumption/api-overview#running-subscription-worker)). |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `run` | `Task` | Starts the subscription worker work of processing batches, receiving the batch processing delegates (see [above](../../../client-api/data-subscriptions/consumption/api-overview#running-subscription-worker)). |
| `run` | `Future[None]` | Starts the subscription worker work of processing batches, receiving the batch processing delegates (see [above](../../../client-api/data-subscriptions/consumption/api-overview#running-subscription-worker)). |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


| Event | Type\Return type | Description |
|--------|:-----|-------------|
| **after\_acknowledgment** | `AfterAcknowledgmentAction` (event) | Event invoked after each time the server acknowledges batch processing progress. |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| **after\_acknowledgment** | `AfterAcknowledgmentAction` (event) | Event invoked after each time the server acknowledges batch processing progress. |
| **after\_acknowledgment** | `Callable[[SubscriptionBatch[_T]], None]` | Event invoked after each time the server acknowledges batch processing progress. |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

|--------|:-----|-------------|
| **after\_acknowledgment** | `AfterAcknowledgmentAction` (event) | Event invoked after each time the server acknowledges batch processing progress. |

| `AfterAcknowledgmentAction` Parameters | | |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe

Suggested change
| `AfterAcknowledgmentAction` Parameters | | |
| `Callable[[SubscriptionBatch[_T]], None]` Parameters | | |

but better

Suggested change
| `AfterAcknowledgmentAction` Parameters | | |
| `after_acknowledgment` Parameters | | |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done (the second one :o)


| Return value | |
| ------------- | ----- |
| `Task` | The worker waits for the task to finish the event processing |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `Task` | The worker waits for the task to finish the event processing |
| `Future[None]` | The worker waits for the task to finish the event processing |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

The task returned by the `run` function will be terminated with an erroneous state, throwing
a `SubscriberErrorException` exception.

* If `SubscriptionWorkerOptions`'s value `IgnoreSubscriberErrors` is set to True, the erroneous

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* If `SubscriptionWorkerOptions`'s value `IgnoreSubscriberErrors` is set to True, the erroneous
* If `SubscriptionWorkerOptions`'s value `ignore_subscriber_errors` is set to True, the erroneous

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

The above cases describe situations in which a worker tries to reconnect with the server.
There are two key `SubscriptionWorkerOptions` fields controlling this state:

* `TimeToWaitBeforeConnectionRetry` - Worker 'sleep' period before trying to reconnect.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `TimeToWaitBeforeConnectionRetry` - Worker 'sleep' period before trying to reconnect.
* `time_to_wait_before_connection_retry` - Worker 'sleep' period before trying to reconnect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

There are two key `SubscriptionWorkerOptions` fields controlling this state:

* `TimeToWaitBeforeConnectionRetry` - Worker 'sleep' period before trying to reconnect.
* `MaxErroneousPeriod` - Maximum time that the worker is allowed to be in an erroneous state.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `MaxErroneousPeriod` - Maximum time that the worker is allowed to be in an erroneous state.
* `max_erroneous_period` - Maximum time that the worker is allowed to be in an erroneous state.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

{INFO/}

{INFO: OnUnexpectedSubscriptionError}
`OnUnexpectedSubscriptionError` is the event raised when a connection failure occurs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`OnUnexpectedSubscriptionError` is the event raised when a connection failure occurs
`on_unexpected_subscription_error` is the event raised when a connection failure occurs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link

@poissoncorp poissoncorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's address the comments & we're good to merge

@ppekrol ppekrol merged commit f4118a0 into ravendb:master Jul 10, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants