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.

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

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

Merged
merged 5 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading