refactor(db): Add TenantID field to KafkaEvent struct #4598
Conversation
subhajit20
changed the title
crates/router/src/services/kafka.rs
Outdated
| ))) | ||
| self.log_event(&KafkaEvent::old( | ||
| &KafkaPaymentAttempt::from_storage(&negative_event), | ||
| TenantID("default".to_string()), |
There was a problem hiding this comment.
can you use the tenant_id stored in the kafkastore instead?
There was a problem hiding this comment.
Yes I have used that TenantID here only ... I have imported that at the top in kafka.rs file
There was a problem hiding this comment.
I mean that can you add a parameter in the log_payment_attempt function to accept a tenant_id and use that instead.
| @@ -30,6 +30,7 @@ use crate::types::storage::Dispute; | |||
|
|
|||
| // Using message queue result here to avoid confusion with Kafka result provided by library | |||
| pub type MQResult<T> = CustomResult<T, KafkaError>; | |||
| use crate::db::kafka_store::TenantID; | |||
There was a problem hiding this comment.
Here is the line @lsampras
crates/router/src/db/kafka_store.rs
Outdated
| @@ -1109,7 +1109,7 @@ impl PaymentAttemptInterface for KafkaStore { | |||
|
|
|||
| if let Err(er) = self | |||
| .kafka_producer | |||
| .log_payment_attempt(&attempt, None) | |||
| .log_payment_attempt(&attempt, None, TenantID("default".to_string())) | |||
There was a problem hiding this comment.
| .log_payment_attempt(&attempt, None, TenantID("default".to_string())) | |
| .log_payment_attempt(&attempt, None, self.tenant_id) |
Can we use the tenant_id stored in KafkaStore struct as such
crates/router/src/services/kafka.rs
Outdated
| self.log_event(&KafkaEvent::old( | ||
| &KafkaPaymentAttempt::from_storage(delete_old_attempt), | ||
| TenantID("default".to_string()), | ||
| )) |
There was a problem hiding this comment.
Can we follow the same pattern as we did for log_payment_attempt function for every other usage as well?
there shouldn't be any new initializations of TenantID in this PR, since it is already defined in KafkaStore which is what we'll be using instead
There was a problem hiding this comment.
Yes, I this commit solves this issue,
let me know if further changes needed. @lsampras
…ducer impl and fixed lint issues
There was a problem hiding this comment.
@subhajit20 the code changes look good, Will you be able to test this out locally & share the screenshots of updated Kafka events?
|
The kafka events seem to have tenant-ID present 
|
lsampras
added
A-framework
|
Thanks for the contribution @subhajit20. |
Type of Change
#4595
Description
Additional Changes
Motivation and Context
How did you test it?
Checklist
cargo +nightly fmt --allcargo clippy