-
Notifications
You must be signed in to change notification settings - Fork 2
Lookup submision IDs by strategic metadata #112
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
base: master
Are you sure you want to change the base?
Changes from all commits
6774d64
9fd6fcd
3235797
bc32ea8
da93710
6f9e1e3
67e7a4c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,10 @@ | |
| ] | ||
|
|
||
|
|
||
| class LookupIdsWithEmptyStrategicMetadataError(Exception): | ||
| pass | ||
|
|
||
|
|
||
| class ProducerClient: | ||
| """ | ||
| Opsqueue producer client. Allows sending of large collections of operations ('submissions') | ||
|
|
@@ -367,6 +371,25 @@ def lookup_submission_id_by_prefix(self, prefix: str) -> SubmissionId | None: | |
| """ | ||
| return self.inner.lookup_submission_id_by_prefix(prefix) | ||
|
|
||
| def lookup_submission_ids_by_strategic_metadata( | ||
| self, strategic_metadata: dict[str, int] | ||
| ) -> list[SubmissionId]: | ||
| """Attempts to find in-progress submissions where the strategic metadata | ||
| of that submission includes all of the key-value pairs of the given | ||
| 'strategic_metadata'. A matching submission must include all of the | ||
| given key-value pairs, but it may also contain other key-value pairs. | ||
|
|
||
| Raises: | ||
| - `LookupIdsWithEmptyStrategicMetadataError` if the provided | ||
| 'strategic_metadata' contained no key-value pairs to look for. | ||
|
Comment on lines
+382
to
+384
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this potentially also raise:
Comment on lines
+382
to
+384
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would change the interface to raise an explicit error if there are too many matching Submissions, we need that for DoS protection anyway, but allow an empty |
||
|
|
||
| """ | ||
| if len(strategic_metadata) == 0: | ||
| raise LookupIdsWithEmptyStrategicMetadataError() | ||
| return self.inner.lookup_submission_ids_by_strategic_metadata( # type: ignore[no-any-return] | ||
| strategic_metadata | ||
| ) | ||
|
|
||
| def is_completed(self, submission_id: SubmissionId) -> bool: | ||
| raise NotImplementedError | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,9 @@ use std::sync::Arc; | |
|
|
||
| use crate::common::errors::E::{L, R}; | ||
| use crate::common::submission::{self, SubmissionId}; | ||
| use crate::common::StrategicMetadataMap; | ||
| use crate::db::{self, DBPools}; | ||
| use axum::extract; | ||
| use axum::extract::{Path, State}; | ||
| use axum::http::StatusCode; | ||
| use axum::response::{IntoResponse, Response}; | ||
|
|
@@ -60,6 +62,10 @@ impl ServerState { | |
| "/submissions/lookup_id_by_prefix/{prefix}", | ||
| get(lookup_submission_id_by_prefix), | ||
| ) | ||
| .route( | ||
| "/submissions/lookup_ids_by_strategic_metadata", | ||
| post(lookup_submission_ids_by_strategic_metadata), | ||
| ) | ||
| .route("/submissions/{submission_id}", get(submission_status)) | ||
| .route("/version", get(crate::server::version_endpoint)) // We're also exposing it here so the producer client can view it | ||
| .with_state(self) | ||
|
|
@@ -133,6 +139,16 @@ async fn lookup_submission_id_by_prefix( | |
| Ok(Json(submission_id)) | ||
| } | ||
|
|
||
| async fn lookup_submission_ids_by_strategic_metadata( | ||
| State(state): State<ServerState>, | ||
| extract::Json(strategic_metadata): extract::Json<StrategicMetadataMap>, | ||
| ) -> Result<Json<Vec<SubmissionId>>, ServerError> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our
For ~1 million this is ~20MB. We can put that as an configurable upper bound to return to the client with an explicit error if it goes over that so that people can update the settings and return the larger response knowingly. |
||
| let mut conn = state.pool.reader_conn().await?; | ||
| let submission_ids = | ||
| submission::db::lookup_ids_by_strategic_metadata(strategic_metadata, &mut conn).await?; | ||
| Ok(Json(submission_ids)) | ||
| } | ||
|
|
||
| #[tracing::instrument(level = "debug", skip(state))] | ||
| async fn insert_submission( | ||
| State(state): State<ServerState>, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the other APIs on the producer and they either provide a single element or an iterator so that it can be lazily evaluated and doesn't need to be materialized in memory all at once.