-
Notifications
You must be signed in to change notification settings - Fork 0
URI resolver for external data fetching #50
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: main
Are you sure you want to change the base?
Changes from all commits
9931447
8440b03
2507112
296bc5b
85ea67d
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 |
|---|---|---|
|
|
@@ -347,13 +347,18 @@ pub fn generate_vm_handler( | |
| }; | ||
|
|
||
| let mut token_requests = Vec::new(); | ||
| let mut url_requests = Vec::new(); | ||
| let mut other_requests = Vec::new(); | ||
|
|
||
| for request in requests { | ||
| match request.resolver { | ||
| match &request.resolver { | ||
| hyperstack::runtime::hyperstack_interpreter::ast::ResolverType::Token => { | ||
| token_requests.push(request) | ||
| } | ||
| hyperstack::runtime::hyperstack_interpreter::ast::ResolverType::Url(_) => { | ||
| url_requests.push(request) | ||
| } | ||
| #[allow(unreachable_patterns)] | ||
| _ => other_requests.push(request), | ||
| } | ||
| } | ||
|
|
@@ -429,6 +434,62 @@ pub fn generate_vm_handler( | |
| } | ||
| } | ||
|
|
||
| // Process URL resolver requests | ||
| if !url_requests.is_empty() { | ||
| let url_client = hyperstack::runtime::hyperstack_interpreter::resolvers::UrlResolverClient::new(); | ||
|
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. We should probably instantiate the client once at startup and attach it onto |
||
|
|
||
| for request in url_requests { | ||
|
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. Should probably do this loop in parallel |
||
| if let hyperstack::runtime::hyperstack_interpreter::ast::ResolverType::Url(config) = &request.resolver { | ||
| // Get the URL from the input value | ||
| let url = match &request.input { | ||
| hyperstack::runtime::serde_json::Value::String(s) => s.clone(), | ||
| _ => { | ||
| hyperstack::runtime::tracing::warn!( | ||
| "URL resolver input is not a string: {:?}", | ||
| request.input | ||
| ); | ||
| let mut vm = self.vm.lock().unwrap_or_else(|e| e.into_inner()); | ||
| vm.restore_resolver_requests(vec![request]); | ||
| continue; | ||
| } | ||
| }; | ||
|
|
||
| if url.is_empty() { | ||
|
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. Should probably requeue here as well: |
||
| continue; | ||
| } | ||
|
|
||
| match url_client.resolve_with_extract(&url, &config.method, config.extract_path.as_deref()).await { | ||
| Ok(resolved_value) => { | ||
| let mut vm = self.vm.lock().unwrap_or_else(|e| e.into_inner()); | ||
| match vm.apply_resolver_result( | ||
| self.bytecode.as_ref(), | ||
| &request.cache_key, | ||
| resolved_value, | ||
| ) { | ||
| Ok(mut new_mutations) => { | ||
| mutations.append(&mut new_mutations); | ||
| } | ||
| Err(err) => { | ||
| hyperstack::runtime::tracing::warn!( | ||
| url = %url, | ||
| "Failed to apply URL resolver result: {}", | ||
| err | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| Err(err) => { | ||
|
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. We should probably requeue these so they get retried. Similar to the token resolver calling
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. Dont want a minor network error to result in missing a value with |
||
| hyperstack::runtime::tracing::warn!( | ||
| url = %url, | ||
| "URL resolver request failed: {}", | ||
| err | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
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. All the updates in this block also apply to the other location that duplicates this logic which is on lines 1400+ below |
||
| if !other_requests.is_empty() { | ||
| let other_count = other_requests.len(); | ||
| let mut vm = self.vm.lock().unwrap_or_else(|e| e.into_inner()); | ||
|
|
@@ -1255,13 +1316,18 @@ pub fn generate_vm_handler_struct() -> TokenStream { | |
| }; | ||
|
|
||
| let mut token_requests = Vec::new(); | ||
| let mut url_requests = Vec::new(); | ||
| let mut other_requests = Vec::new(); | ||
|
|
||
| for request in requests { | ||
| match request.resolver { | ||
| match &request.resolver { | ||
| hyperstack::runtime::hyperstack_interpreter::ast::ResolverType::Token => { | ||
| token_requests.push(request) | ||
| } | ||
| hyperstack::runtime::hyperstack_interpreter::ast::ResolverType::Url(_) => { | ||
| url_requests.push(request) | ||
| } | ||
| #[allow(unreachable_patterns)] | ||
| _ => other_requests.push(request), | ||
| } | ||
| } | ||
|
|
@@ -1337,6 +1403,62 @@ pub fn generate_vm_handler_struct() -> TokenStream { | |
| } | ||
| } | ||
|
|
||
| // Process URL resolver requests | ||
| if !url_requests.is_empty() { | ||
| let url_client = hyperstack::runtime::hyperstack_interpreter::resolvers::UrlResolverClient::new(); | ||
|
|
||
| for request in url_requests { | ||
| if let hyperstack::runtime::hyperstack_interpreter::ast::ResolverType::Url(config) = &request.resolver { | ||
| // Get the URL from the input value | ||
| let url = match &request.input { | ||
| hyperstack::runtime::serde_json::Value::String(s) => s.clone(), | ||
| _ => { | ||
| hyperstack::runtime::tracing::warn!( | ||
| "URL resolver input is not a string: {:?}", | ||
| request.input | ||
| ); | ||
| let mut vm = self.vm.lock().unwrap_or_else(|e| e.into_inner()); | ||
| vm.restore_resolver_requests(vec![request]); | ||
| continue; | ||
| } | ||
| }; | ||
|
|
||
| if url.is_empty() { | ||
| continue; | ||
| } | ||
|
|
||
| match url_client.resolve_with_extract(&url, &config.method, config.extract_path.as_deref()).await { | ||
| Ok(resolved_value) => { | ||
| let mut vm = self.vm.lock().unwrap_or_else(|e| e.into_inner()); | ||
| match vm.apply_resolver_result( | ||
| self.bytecode.as_ref(), | ||
| &request.cache_key, | ||
| resolved_value, | ||
| ) { | ||
| Ok(mut new_mutations) => { | ||
| mutations.append(&mut new_mutations); | ||
| } | ||
| Err(err) => { | ||
| hyperstack::runtime::tracing::warn!( | ||
| url = %url, | ||
| "Failed to apply URL resolver result: {}", | ||
| err | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| Err(err) => { | ||
| hyperstack::runtime::tracing::warn!( | ||
| url = %url, | ||
| "URL resolver request failed: {}", | ||
| err | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !other_requests.is_empty() { | ||
| let other_count = other_requests.len(); | ||
| let mut vm = self.vm.lock().unwrap_or_else(|e| e.into_inner()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1004,6 +1004,8 @@ pub fn parse_aggregate_attribute( | |
| pub struct ResolveAttribute { | ||
| pub from: Option<String>, | ||
| pub address: Option<String>, | ||
| pub url: Option<String>, | ||
| pub method: Option<String>, | ||
| pub extract: Option<String>, | ||
| pub target_field_name: String, | ||
| pub resolver: Option<String>, | ||
|
|
@@ -1023,6 +1025,8 @@ pub struct ResolveSpec { | |
| struct ResolveAttributeArgs { | ||
| from: Option<String>, | ||
| address: Option<String>, | ||
| url: Option<String>, | ||
| method: Option<syn::Ident>, | ||
| extract: Option<String>, | ||
| resolver: Option<String>, | ||
| strategy: Option<String>, | ||
|
|
@@ -1032,6 +1036,8 @@ impl Parse for ResolveAttributeArgs { | |
| fn parse(input: ParseStream) -> syn::Result<Self> { | ||
| let mut from = None; | ||
| let mut address = None; | ||
| let mut url = None; | ||
| let mut method = None; | ||
| let mut extract = None; | ||
| let mut resolver = None; | ||
| let mut strategy = None; | ||
|
|
@@ -1048,6 +1054,22 @@ impl Parse for ResolveAttributeArgs { | |
| } else if ident_str == "address" { | ||
| let lit: syn::LitStr = input.parse()?; | ||
| address = Some(lit.value()); | ||
| } else if ident_str == "url" { | ||
| // Parse as dotted path (e.g., info.uri) - handle both dot-separated and single identifiers | ||
| let mut parts = Vec::new(); | ||
| let first: syn::Ident = input.parse()?; | ||
| parts.push(first.to_string()); | ||
|
|
||
| // Parse any additional .identifier segments | ||
| while input.peek(Token![.]) { | ||
| input.parse::<Token![.]>()?; | ||
| let next: syn::Ident = input.parse()?; | ||
| parts.push(next.to_string()); | ||
| } | ||
|
|
||
| url = Some(parts.join(".")); | ||
| } else if ident_str == "method" { | ||
| method = Some(input.parse()?); | ||
|
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. we should probably fail for any mehtod that isn't |
||
| } else if ident_str == "extract" { | ||
| let lit: syn::LitStr = input.parse()?; | ||
| extract = Some(lit.value()); | ||
|
|
@@ -1077,6 +1099,8 @@ impl Parse for ResolveAttributeArgs { | |
| Ok(ResolveAttributeArgs { | ||
| from, | ||
| address, | ||
| url, | ||
| method, | ||
| extract, | ||
| resolver, | ||
| strategy, | ||
|
|
@@ -1094,17 +1118,37 @@ pub fn parse_resolve_attribute( | |
|
|
||
| let args: ResolveAttributeArgs = attr.parse_args()?; | ||
|
|
||
| // Check for mutually exclusive parameters: url vs (from/address) | ||
| let has_url = args.url.is_some(); | ||
| let has_token_source = args.from.is_some() || args.address.is_some(); | ||
|
|
||
| if has_url && has_token_source { | ||
| return Err(syn::Error::new_spanned( | ||
| attr, | ||
| "#[resolve] cannot specify 'url' together with 'from' or 'address'", | ||
| )); | ||
| } | ||
|
|
||
| if !has_url && !has_token_source { | ||
| return Err(syn::Error::new_spanned( | ||
| attr, | ||
| "#[resolve] requires either 'url' or 'from'/'address' parameter", | ||
| )); | ||
| } | ||
|
|
||
| // Token resolvers: cannot have both from and address | ||
| if args.from.is_some() && args.address.is_some() { | ||
| return Err(syn::Error::new_spanned( | ||
| attr, | ||
| "#[resolve] cannot specify both 'from' and 'address'", | ||
| )); | ||
| } | ||
|
|
||
| if args.from.is_none() && args.address.is_none() { | ||
| // URL resolvers require extract parameter | ||
| if has_url && args.extract.is_none() { | ||
| return Err(syn::Error::new_spanned( | ||
| attr, | ||
| "#[resolve] requires either 'from' or 'address' parameter", | ||
| "#[resolve] with 'url' requires 'extract' parameter", | ||
| )); | ||
| } | ||
|
|
||
|
|
@@ -1113,6 +1157,8 @@ pub fn parse_resolve_attribute( | |
| Ok(Some(ResolveAttribute { | ||
| from: args.from, | ||
| address: args.address, | ||
| url: args.url, | ||
| method: args.method.map(|m| m.to_string()), | ||
| extract: args.extract, | ||
| target_field_name: target_field_name.to_string(), | ||
| resolver: args.resolver, | ||
|
|
@@ -1146,7 +1192,6 @@ pub fn parse_computed_attribute( | |
| target_field_name: target_field_name.to_string(), | ||
| })) | ||
| } | ||
|
|
||
| pub fn has_entity_attribute(attrs: &[Attribute]) -> bool { | ||
| attrs.iter().any(|attr| attr.path().is_ident("entity")) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,9 +208,16 @@ fn build_resolver_specs(resolve_specs: &[parse::ResolveSpec]) -> Vec<ResolverSpe | |
| extracts: Vec::new(), | ||
| }); | ||
|
|
||
| // For URL resolvers with extract_path, the value is already extracted by | ||
| // resolve_with_extract, so source_path should be None to use the value directly | ||
| let source_path = match &spec.resolver { | ||
|
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. The source_path = None branch for Url resolvers encodes knowledge that extraction happens inside UrlResolverClient rather than in apply_resolver_result. This couples the AST builder to a client implementation detail. The cleaner fix is to have UrlResolverClient return the raw JSON response and let apply_resolver_result do extraction via source_path — exactly how token resolvers work. Then this special case disappears and extraction is uniform across all resolver types |
||
| ResolverType::Url(config) if config.extract_path.is_some() => None, | ||
| _ => spec.extract.clone(), | ||
| }; | ||
|
|
||
| let extract = ResolverExtractSpec { | ||
| target_path: spec.target_field_name.clone(), | ||
| source_path: spec.extract.clone(), | ||
| source_path, | ||
| transform: None, | ||
| }; | ||
|
|
||
|
|
@@ -232,9 +239,10 @@ fn parse_resolve_strategy(strategy: &str) -> ResolveStrategy { | |
| } | ||
| } | ||
|
|
||
| fn resolver_type_key(resolver: &ResolverType) -> &'static str { | ||
| fn resolver_type_key(resolver: &ResolverType) -> String { | ||
| match resolver { | ||
| ResolverType::Token => "token", | ||
| ResolverType::Token => "token".to_string(), | ||
| ResolverType::Url(config) => format!("url:{}", config.url_path), | ||
| } | ||
| } | ||
|
|
||
|
|
||
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've noticed these types are duplicated in the interpreter as well. My code from before is also duplicated. Not worth fixing now but worth making a note to consolidate those over time