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.

Sign up for GitHub

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

Jump to bottom

feat: pin to working protos version, add lockfile #124

Merged
merged 9 commits into from
Feb 15, 2023
Merged

feat: pin to working protos version, add lockfile #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
04fb416
feat: pin to working protos version, add lockfile
cprice404 Feb 14, 2023
8647814
fix: try upgrading chrono to get rid of vulnerable time dep
cprice404 Feb 14, 2023
f5c2add
chore: get rid of chrono dependency
cprice404 Feb 15, 2023
7f9b17b
fix: linting
cprice404 Feb 15, 2023
80c1e38
fix: clippy
cprice404 Feb 15, 2023
07b0e46
fix: clippy
cprice404 Feb 15, 2023
667226e
chore: run on ubuntu 20.04 instead of latest due to network issues
cprice404 Feb 15, 2023
1afc464
chore: put the cargo.lock file back into .gitignore
cprice404 Feb 15, 2023
14f1c06
Revert "chore: run on ubuntu 20.04 instead of latest due to network i…
cprice404 Feb 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ members = [
exclude = [ "example" ]

[dependencies]
momento-protos = { version = "0.42" }
momento-protos = { version = "=0.42.4" }
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really think it makes sense to pin to explicit versions like this. Semantically the SDK depends on momento protos 0.42, and the lockfile is the better tool for explicit versions.

I'm not going to block you on this and I will approve once the approvals are green but I don't agree with this style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right now if i don't do this, it literally won't compile because 0.42.5 is not compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

you could put the version lock in the new cargo.lock file like this

[[package]]
name = "momento-protos"
version = "0.42.4"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i was not aware of / under the impression that it was fair game to make manual edits to that file.

Copy link
Contributor

Choose a reason for hiding this comment

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

One can edit any file one wishes to edit. It's certainly atypical, but it's also quite atypical to publish a library without a cargo.lock file :-(

As I said, I'm not going to block you but I disagree with this plan.

log = "0.4.17"
hyper = { version = "0.14" }
h2 = { version = "0.3" }
Expand All @@ -27,7 +27,6 @@ jsonwebtoken = "8.0.1"
rand = "0.8.5"
serde = {version = "1.0", features = ["derive"] }
serde_json = "1.0.79"
chrono = {version = "0.4.19", features = ["serde"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

hooray!

thiserror = "1.0.38"

[dev-dependencies]
Expand Down
4 changes: 2 additions & 2 deletions src/response/create_signing_key_response.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use chrono::Utc;
use serde::{Deserialize, Serialize};
use std::time::SystemTime;

/// The results of a singing key operation.
#[derive(Debug, Serialize, Deserialize)]
Expand All @@ -9,7 +9,7 @@ pub struct MomentoCreateSigningKeyResponse {
/// Key itself
pub key: String,
/// When the key expires
pub expires_at: chrono::DateTime<Utc>,
pub expires_at: SystemTime,
/// Endpoint for creating a pre-signed url
pub endpoint: String,
}
4 changes: 2 additions & 2 deletions src/response/list_signing_keys_response.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use chrono::Utc;
use serde::{Deserialize, Serialize};
use std::time::SystemTime;

/// Response signing key for list of signing keys.
#[derive(Debug, Serialize, Deserialize)]
pub struct MomentoSigningKey {
pub key_id: String,
pub expires_at: chrono::DateTime<Utc>,
pub expires_at: SystemTime,
pub endpoint: String,
}

Expand Down
15 changes: 3 additions & 12 deletions src/simple_cache_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use chrono::{DateTime, NaiveDateTime, Utc};
use momento_protos::{
cache_client::scs_client::*,
cache_client::*,
Expand All @@ -11,7 +10,7 @@ use serde_json::Value;
use std::convert::TryFrom;
use std::iter::FromIterator;
use std::ops::RangeBounds;
use std::time::Duration;
use std::time::{Duration, UNIX_EPOCH};
use std::{
collections::{HashMap, HashSet},
convert::TryInto,
Expand Down Expand Up @@ -395,11 +394,7 @@ impl SimpleCacheClient {
let response = MomentoCreateSigningKeyResponse {
key_id: kid.as_str().expect("'kid' not a valid str").to_owned(),
key: res.key,
expires_at: DateTime::<Utc>::from_utc(
NaiveDateTime::from_timestamp_opt(res.expires_at as i64, 0)
.expect("couldn't parse from timestamp"),
Utc,
),
expires_at: UNIX_EPOCH + Duration::from_secs(res.expires_at),
endpoint: self.data_endpoint.clone(),
};
Ok(response)
Expand Down Expand Up @@ -465,11 +460,7 @@ impl SimpleCacheClient {
.iter()
.map(|signing_key| MomentoSigningKey {
key_id: signing_key.key_id.to_string(),
expires_at: DateTime::<Utc>::from_utc(
NaiveDateTime::from_timestamp_opt(signing_key.expires_at as i64, 0)
.expect("couldn't parse timestamp from signing key"),
Utc,
),
expires_at: UNIX_EPOCH + Duration::from_secs(signing_key.expires_at),
endpoint: self.data_endpoint.clone(),
})
.collect();
Expand Down