Skip to content

Commit

Permalink
chore: warn when the 'canister_ids.json' file is first generated for …
Browse files Browse the repository at this point in the history
…persistent networks. (#4058)

* Warn when the 'canister_ids.json' file is first generated for persistent networks.

* Add e2e tests.

* Update changelog.
  • Loading branch information
vincent-dfinity authored Jan 9, 2025
1 parent 72f0cde commit 55f23c2
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 8 deletions.
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,19 @@ Your principal for ICP wallets and decentralized exchanges: ueuar-wxbnk-bdcsr-dn

Add pre-install tasks, which can be defined by the new `pre-install` key for canister objects in `dfx.json` with a command or list of commands.

### chore: Warn when the 'canister_ids.json' file is first generated for persistent networks.

Warn when the 'canister_ids.json' file is first generated for persistent networks.

```
dfx deploy --network ic
...
test_backend canister created on network ic with canister id: j36qm-pqaaa-aaaan-qzqya-cai
WARN: The "/home/sdk/repos/test/canister_ids.json" file has been generated. Please make sure you store it correctly, e.g., submitting it to a GitHub repository.
Building canisters...
...
```

## Dependencies

### Frontend canister
Expand Down
2 changes: 2 additions & 0 deletions e2e/tests-dfx/network.bash
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ teardown() {
dfx_start

assert_command dfx canister create --all --network local
assert_not_contains "canister_ids.json\" file has been generated. Please make sure you store it correctly"

# canister creates writes to a spinner (stderr), not stdout
assert_command dfx canister id e2e_project_backend --network local
Expand All @@ -64,6 +65,7 @@ teardown() {
jq '.local.type="persistent"' "$E2E_NETWORKS_JSON" | sponge "$E2E_NETWORKS_JSON"

assert_command dfx canister create --all --network local
assert_contains "canister_ids.json\" file has been generated. Please make sure you store it correctly"

# canister creates writes to a spinner (stderr), not stdout
assert_command dfx canister id e2e_project_backend --network local
Expand Down
36 changes: 31 additions & 5 deletions src/dfx-core/src/config/model/canister_id_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,16 +215,37 @@ impl CanisterIdStore {
.map(|(canister_name, _)| canister_name)
}

pub fn save_ids(&self) -> Result<(), SaveIdsError> {
fn warn_for_canister_ids_path(&self) -> bool {
// Only warn when the 'canister_ids.json' file is first generated under the project root directory for persistent networks.
if let NetworkDescriptor {
r#type: NetworkTypeDescriptor::Persistent,
..
} = self.network_descriptor
{
if let Some(path) = self.canister_ids_path.as_ref() {
if !path.exists() {
return true;
}
}
};

false
}

pub fn save_ids(&self, log: &Logger) -> Result<(), SaveIdsError> {
let path = self
.canister_ids_path
.as_ref()
.unwrap_or_else(|| {
// the only callers of this method have already called Environment::get_config_or_anyhow
unreachable!("Must be in a project (call Environment::get_config_or_anyhow()) to save canister ids")
});
let to_warn = self.warn_for_canister_ids_path();
crate::fs::composite::ensure_parent_dir_exists(path)?;
crate::json::save_json_file(path, &self.ids)?;
if to_warn {
warn!(log, "The {:?} file has been generated. Please make sure you store it correctly, e.g., submitting it to a GitHub repository.", path);
}
Ok(())
}

Expand Down Expand Up @@ -309,6 +330,7 @@ impl CanisterIdStore {

pub fn add(
&mut self,
log: &Logger,
canister_name: &str,
canister_id: &str,
timestamp: Option<AcquisitionDateTime>,
Expand All @@ -327,7 +349,7 @@ impl CanisterIdStore {
.insert(canister_name.to_string(), network_name_to_canister_id);
}
}
self.save_ids()
self.save_ids(log)
.map_err(|source| AddCanisterIdError::SaveIds {
canister_name: canister_name.to_string(),
canister_id: canister_id.to_string(),
Expand All @@ -350,11 +372,15 @@ impl CanisterIdStore {
Ok(())
}

pub fn remove(&mut self, canister_name: &str) -> Result<(), RemoveCanisterIdError> {
pub fn remove(
&mut self,
log: &Logger,
canister_name: &str,
) -> Result<(), RemoveCanisterIdError> {
let network_name = &self.network_descriptor.name;
if let Some(network_name_to_canister_id) = self.ids.get_mut(canister_name) {
network_name_to_canister_id.remove(network_name);
self.save_ids()
self.save_ids(log)
.map_err(|e| RemoveCanisterIdError::SaveIds {
canister_name: canister_name.to_string(),
source: e,
Expand Down Expand Up @@ -392,7 +418,7 @@ impl CanisterIdStore {

for canister in canisters_to_prune {
warn!(log, "Canister '{}' has timed out.", &canister);
self.remove(&canister)?;
self.remove(log, &canister)?;
}

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion src/dfx/src/commands/canister/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ async fn delete_canister(
}

if let Some(canister_name) = canister_name_to_delete {
canister_id_store.remove(&canister_name)?;
canister_id_store.remove(log, &canister_name)?;
}

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion src/dfx/src/lib/named_canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub async fn install_ui_canister(
.with_mode(InstallMode::Install)
.await
.context("Install wasm call failed.")?;
id_store.add(UI_CANISTER, &canister_id.to_text(), None)?;
id_store.add(env.get_logger(), UI_CANISTER, &canister_id.to_text(), None)?;
info!(
env.get_logger(),
"The UI canister on the \"{}\" network is \"{}\"",
Expand Down
2 changes: 1 addition & 1 deletion src/dfx/src/lib/operations/canister/create_canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ The command line value will be used.",
non_default_network,
canister_id
);
canister_id_store.add(canister_name, &canister_id, None)?;
canister_id_store.add(log, canister_name, &canister_id, None)?;

Ok(())
}
Expand Down
1 change: 1 addition & 0 deletions src/dfx/src/lib/operations/canister/install_canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ The command line value will be used.",
)
.await?;
canister_id_store.add(
log,
canister_info.get_name(),
&canister_id.to_string(),
Some(new_timestamp),
Expand Down
1 change: 1 addition & 0 deletions src/dfx/src/lib/operations/canister/motoko_playground.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ pub async fn reserve_canister_with_playground(
.context("Failed to reserve canister at the playground.")?;
let reserved_canister = Decode!(&result, CanisterInfo)?;
canister_id_store.add(
log,
canister_name,
&reserved_canister.id.to_string(),
Some(reserved_canister.get_timestamp()?),
Expand Down

0 comments on commit 55f23c2

Please sign in to comment.