posix4e / rust-metrics

Multi reporter metrics library (carbon, graphite, postgresql, prometheus)
Other
99 stars 19 forks source link

Final bits of remove #67

Closed eb4890 closed 7 years ago

eb4890 commented 7 years ago

This should clean up all the removal stuff for now.

posix4e commented 7 years ago

Looks like the tests are failing

eb4890 commented 7 years ago

You need to release the new version of prometheus reporter to cranesbill

On 8 Dec 2016 16:31, "Alex Newman" notifications@github.com wrote:

Looks like the tests are failing

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/posix4e/rust-metrics/pull/67#issuecomment-265785353, or mute the thread https://github.com/notifications/unsubscribe-auth/AAiLrVmODJsTisNlhPYamDkP4Hv0fggsks5rGDDmgaJpZM4LHH2s .

posix4e commented 7 years ago

@eb4890 Having some issues

posix4e commented 7 years ago
Compiling prometheus_reporter v0.0.2 (file:///home/posi/src/rust-metrics/prometheus_reporter/target/package/prometheus_reporter-0.0.2)
target/package/prometheus_reporter-0.0.2/src/lib.rs:100:38: 100:56 error: no associated item named `new` found for type `std::collections::hash_map::DefaultHasher` in the current scope
target/package/prometheus_reporter-0.0.2/src/lib.rs:100                     let mut hasher = DefaultHasher::new();
                                                                                             ^~~~~~~~~~~~~~~~~~
target/package/prometheus_reporter-0.0.2/src/lib.rs:112:26: 112:44 error: no associated item named `new` found for type `std::collections::hash_map::DefaultHasher` in the current scope
target/package/prometheus_reporter-0.0.2/src/lib.rs:112         let mut hasher = DefaultHasher::new();
                                                                                 ^~~~~~~~~~~~~~~~~~
target/package/prometheus_reporter-0.0.2/src/lib.rs:118:38: 118:56 error: no associated item named `new` found for type `std::collections::hash_map::DefaultHasher` in the current scope
target/package/prometheus_reporter-0.0.2/src/lib.rs:118                     let mut hasher = DefaultHasher::new();
                                                                                             ^~~~~~~~~~~~~~~~~~
error: aborting due to 3 previous errors
error: failed to verify package tarball

Caused by:
  Could not compile `prometheus_reporter`.

To learn more, run the command again with --verbose.
posi@posi:~/src/rust-metrics/prometheus_reporter$ git status
On branch final_bits_of_remove
Your branch is up-to-date with 'eb4890/final_bits_of_remove'.
nothing to commit, working directory clean
posi@posi:~/src/rust-metrics/prometheus_reporter$ git show HEAD
commit 79e75df137569dd49b6c4dd1db2e37d2aa4314f6
posix4e commented 7 years ago

Let me try upgrading my version of rust

posix4e commented 7 years ago

I still don't understand

    pub fn remove(&mut self, metrics_to_remove: Vec<String>) -> Result<i64, String> {
        let hasher = DefaultHasher::new();
        match self.cache.write() {
            Ok(mut cache) => {
                let mut counter = 0;
                for metric_name_to_remove in metrics_to_remove {

                    let mut hasher = DefaultHasher::new();
                    metric_name_to_remove.hash(&mut hasher);
                    if let Some(_) = cache.remove(&hasher.finish()) {
                        counter = counter + 1;
                    }
                }
                Ok(counter)
            }
            Err(y) => Err(format!("Unable to remove {}", y)),
        }
    }
}

It seems both of the hashers are unused. Or maybe one of them?

posix4e commented 7 years ago

OK sorry I forgot I had to upgrade, I can publish the crate but I still don't understand how the above function is to work. Maybe for now have it go off the local path, and we can release both crates at the same time?

eb4890 commented 7 years ago

Ah yeah the top one is unused. Sorry I'll update the PR to modify that as well. It was left in from a point in time where I didn't know where hashers worked.

eb4890 commented 7 years ago

As to how it works. I changed the add function to also use the hash of the metric name as the key, so you can just remove the metric with the same name and hashing.