Skip to content

Commit

Permalink
Merge pull request #25243 from antiguru/enable_warn_missing_docs
Browse files Browse the repository at this point in the history
  • Loading branch information
antiguru authored Feb 14, 2024
2 parents bcf803b + 4ebf203 commit f7cd1fe
Show file tree
Hide file tree
Showing 20 changed files with 176 additions and 10 deletions.
4 changes: 3 additions & 1 deletion src/compute-client/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ fn main() {
env::set_var("PROTOC", protobuf_src::protoc());

let mut config = prost_build::Config::new();
config.btree_map(["."]);
config
.btree_map(["."])
.type_attribute(".", "#[allow(missing_docs)]");

tonic_build::configure()
// Enabling `emit_rerun_if_changed` will rerun the build script when
Expand Down
16 changes: 15 additions & 1 deletion src/compute-client/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,18 @@ pub enum ComputeControllerResponse<T> {
/// See [`ComputeResponse::SubscribeResponse`].
SubscribeResponse(GlobalId, SubscribeBatch<T>),
/// See [`ComputeResponse::FrontierUpper`]
FrontierUpper { id: GlobalId, upper: Antichain<T> },
FrontierUpper {
/// TODO(#25239): Add documentation.
id: GlobalId,
/// TODO(#25239): Add documentation.
upper: Antichain<T>,
},
}

/// Replica configuration
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct ComputeReplicaConfig {
/// TODO(#25239): Add documentation.
pub logging: ComputeReplicaLogging,
/// The amount of effort to be spent on arrangement compaction during idle times.
///
Expand Down Expand Up @@ -188,6 +194,7 @@ impl<T: Timestamp> ComputeController<T> {
}
}

/// TODO(#25239): Add documentation.
pub fn instance_exists(&self, id: ComputeInstanceId) -> bool {
self.instances.contains_key(&id)
}
Expand Down Expand Up @@ -223,6 +230,7 @@ impl<T: Timestamp> ComputeController<T> {
Ok(collection)
}

/// TODO(#25239): Add documentation.
pub fn find_collection(
&self,
collection_id: GlobalId,
Expand Down Expand Up @@ -256,18 +264,22 @@ impl<T: Timestamp> ComputeController<T> {
.collection_reverse_dependencies(id))
}

/// TODO(#25239): Add documentation.
pub fn set_default_idle_arrangement_merge_effort(&mut self, value: u32) {
self.default_idle_arrangement_merge_effort = value;
}

/// TODO(#25239): Add documentation.
pub fn set_default_arrangement_exert_proportionality(&mut self, value: u32) {
self.default_arrangement_exert_proportionality = value;
}

/// TODO(#25239): Add documentation.
pub fn enable_aggressive_readhold_downgrades(&self) -> bool {
self.enable_aggressive_readhold_downgrades
}

/// TODO(#25239): Add documentation.
pub fn set_enable_aggressive_readhold_downgrades(&mut self, value: bool) {
self.enable_aggressive_readhold_downgrades = value;
}
Expand Down Expand Up @@ -443,6 +455,7 @@ pub struct ActiveComputeController<'a, T> {
}

impl<T: Timestamp> ActiveComputeController<'_, T> {
/// TODO(#25239): Add documentation.
pub fn instance_exists(&self, id: ComputeInstanceId) -> bool {
self.compute.instance_exists(id)
}
Expand Down Expand Up @@ -822,6 +835,7 @@ impl<T: Timestamp> CollectionState<T> {
}
}

/// TODO(#25239): Add documentation.
pub fn new_log_collection() -> Self {
let since = Antichain::from_elem(Timestamp::minimum());
let mut state = Self::new(since, Vec::new(), Vec::new());
Expand Down
27 changes: 26 additions & 1 deletion src/compute-client/src/controller/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ pub struct CollectionMissing(pub GlobalId);
/// Errors arising during compute collection lookup.
#[derive(Error, Debug)]
pub enum CollectionLookupError {
/// TODO(#25239): Add documentation.
#[error("instance does not exist: {0}")]
InstanceMissing(ComputeInstanceId),
/// TODO(#25239): Add documentation.
#[error("collection does not exist: {0}")]
CollectionMissing(GlobalId),
}
Expand All @@ -62,10 +64,13 @@ impl From<CollectionMissing> for CollectionLookupError {
/// Errors arising during compute replica creation.
#[derive(Error, Debug)]
pub enum ReplicaCreationError {
/// TODO(#25239): Add documentation.
#[error("instance does not exist: {0}")]
InstanceMissing(ComputeInstanceId),
/// TODO(#25239): Add documentation.
#[error("replica exists already: {0}")]
ReplicaExists(ReplicaId),
/// TODO(#25239): Add documentation.
#[error("collection does not exist: {0}")]
CollectionMissing(GlobalId),
}
Expand All @@ -91,8 +96,10 @@ impl From<CollectionMissing> for ReplicaCreationError {
/// Errors arising during compute replica removal.
#[derive(Error, Debug)]
pub enum ReplicaDropError {
/// TODO(#25239): Add documentation.
#[error("instance does not exist: {0}")]
InstanceMissing(ComputeInstanceId),
/// TODO(#25239): Add documentation.
#[error("replica does not exist: {0}")]
ReplicaMissing(ReplicaId),
}
Expand All @@ -112,12 +119,16 @@ impl From<instance::ReplicaMissing> for ReplicaDropError {
/// Errors arising during dataflow creation.
#[derive(Error, Debug)]
pub enum DataflowCreationError {
/// TODO(#25239): Add documentation.
#[error("instance does not exist: {0}")]
InstanceMissing(ComputeInstanceId),
/// TODO(#25239): Add documentation.
#[error("collection does not exist: {0}")]
CollectionMissing(GlobalId),
/// TODO(#25239): Add documentation.
#[error("dataflow definition lacks an as_of value")]
MissingAsOf,
/// TODO(#25239): Add documentation.
#[error("dataflow has an as_of not beyond the since of collection: {0}")]
SinceViolation(GlobalId),
}
Expand All @@ -142,12 +153,16 @@ impl From<instance::DataflowCreationError> for DataflowCreationError {
/// Errors arising during peek processing.
#[derive(Error, Debug)]
pub enum PeekError {
/// TODO(#25239): Add documentation.
#[error("instance does not exist: {0}")]
InstanceMissing(ComputeInstanceId),
/// TODO(#25239): Add documentation.
#[error("collection does not exist: {0}")]
CollectionMissing(GlobalId),
/// TODO(#25239): Add documentation.
#[error("replica does not exist: {0}")]
ReplicaMissing(ReplicaId),
/// TODO(#25239): Add documentation.
#[error("peek timestamp is not beyond the since of collection: {0}")]
SinceViolation(GlobalId),
}
Expand All @@ -172,8 +187,10 @@ impl From<instance::PeekError> for PeekError {
/// Errors arising during collection updates.
#[derive(Error, Debug)]
pub enum CollectionUpdateError {
/// TODO(#25239): Add documentation.
#[error("instance does not exist: {0}")]
InstanceMissing(ComputeInstanceId),
/// TODO(#25239): Add documentation.
#[error("collection does not exist: {0}")]
CollectionMissing(GlobalId),
}
Expand All @@ -193,10 +210,13 @@ impl From<CollectionMissing> for CollectionUpdateError {
/// Errors arising during collection read policy assignment.
#[derive(Error, Debug)]
pub enum ReadPolicyError {
/// TODO(#25239): Add documentation.
#[error("instance does not exist: {0}")]
InstanceMissing(ComputeInstanceId),
/// TODO(#25239): Add documentation.
#[error("collection does not exist: {0}")]
CollectionMissing(GlobalId),
/// TODO(#25239): Add documentation.
#[error("collection is write-only: {0}")]
WriteOnlyCollection(GlobalId),
}
Expand All @@ -217,15 +237,19 @@ impl From<instance::ReadPolicyError> for ReadPolicyError {
}
}

// Errors arising during subscribe target assignment.
/// Errors arising during subscribe target assignment.
#[derive(Error, Debug)]
pub enum SubscribeTargetError {
/// TODO(#25239): Add documentation.
#[error("instance does not exist: {0}")]
InstanceMissing(ComputeInstanceId),
/// TODO(#25239): Add documentation.
#[error("subscribe does not exist: {0}")]
SubscribeMissing(GlobalId),
/// TODO(#25239): Add documentation.
#[error("replica does not exist: {0}")]
ReplicaMissing(ReplicaId),
/// TODO(#25239): Add documentation.
#[error("subscribe has already produced output")]
SubscribeAlreadyStarted,
}
Expand All @@ -250,6 +274,7 @@ impl From<instance::SubscribeTargetError> for SubscribeTargetError {
/// Errors arising during orphan removal.
#[derive(Error, Debug)]
pub enum RemoveOrphansError {
/// TODO(#25239): Add documentation.
#[error("orchestrator error: {0}")]
OrchestratorError(anyhow::Error),
}
Expand Down
4 changes: 1 addition & 3 deletions src/compute-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0.

// This appears to be defective at the moment, with false positives
// for each variant of the `Command` enum, each of which are documented.
// #![warn(missing_docs)]
#![warn(missing_docs)]

//! The public API for the compute layer.
Expand Down
38 changes: 38 additions & 0 deletions src/compute-client/src/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,16 @@ impl ProtoMapEntry<LogVariant, GlobalId> for ProtoIndexLog {
}
}

/// TODO(#25239): Add documentation.
#[derive(
Arbitrary, Hash, Eq, PartialEq, Ord, PartialOrd, Debug, Clone, Copy, Serialize, Deserialize,
)]
pub enum LogVariant {
/// TODO(#25239): Add documentation.
Timely(TimelyLog),
/// TODO(#25239): Add documentation.
Differential(DifferentialLog),
/// TODO(#25239): Add documentation.
Compute(ComputeLog),
}

Expand Down Expand Up @@ -151,20 +155,32 @@ impl RustType<ProtoLogVariant> for LogVariant {
}
}

/// TODO(#25239): Add documentation.
#[derive(
Arbitrary, Hash, Eq, Ord, PartialEq, PartialOrd, Debug, Clone, Copy, Serialize, Deserialize,
)]
pub enum TimelyLog {
/// TODO(#25239): Add documentation.
Operates,
/// TODO(#25239): Add documentation.
Channels,
/// TODO(#25239): Add documentation.
Elapsed,
/// TODO(#25239): Add documentation.
Histogram,
/// TODO(#25239): Add documentation.
Addresses,
/// TODO(#25239): Add documentation.
Parks,
/// TODO(#25239): Add documentation.
MessagesSent,
/// TODO(#25239): Add documentation.
MessagesReceived,
/// TODO(#25239): Add documentation.
Reachability,
/// TODO(#25239): Add documentation.
BatchesSent,
/// TODO(#25239): Add documentation.
BatchesReceived,
}

Expand Down Expand Up @@ -207,16 +223,24 @@ impl RustType<ProtoTimelyLog> for TimelyLog {
}
}

/// TODO(#25239): Add documentation.
#[derive(
Arbitrary, Hash, Eq, Ord, PartialEq, PartialOrd, Debug, Clone, Copy, Serialize, Deserialize,
)]
pub enum DifferentialLog {
/// TODO(#25239): Add documentation.
ArrangementBatches,
/// TODO(#25239): Add documentation.
ArrangementRecords,
/// TODO(#25239): Add documentation.
Sharing,
/// TODO(#25239): Add documentation.
BatcherRecords,
/// TODO(#25239): Add documentation.
BatcherSize,
/// TODO(#25239): Add documentation.
BatcherCapacity,
/// TODO(#25239): Add documentation.
BatcherAllocations,
}

Expand Down Expand Up @@ -253,20 +277,32 @@ impl RustType<ProtoDifferentialLog> for DifferentialLog {
}
}

/// TODO(#25239): Add documentation.
#[derive(
Arbitrary, Hash, Eq, PartialEq, Ord, PartialOrd, Debug, Clone, Copy, Serialize, Deserialize,
)]
pub enum ComputeLog {
/// TODO(#25239): Add documentation.
DataflowCurrent,
/// TODO(#25239): Add documentation.
FrontierCurrent,
/// TODO(#25239): Add documentation.
PeekCurrent,
/// TODO(#25239): Add documentation.
PeekDuration,
/// TODO(#25239): Add documentation.
FrontierDelay,
/// TODO(#25239): Add documentation.
ImportFrontierCurrent,
/// TODO(#25239): Add documentation.
ArrangementHeapSize,
/// TODO(#25239): Add documentation.
ArrangementHeapCapacity,
/// TODO(#25239): Add documentation.
ArrangementHeapAllocations,
/// TODO(#25239): Add documentation.
ShutdownDuration,
/// TODO(#25239): Add documentation.
ErrorCount,
}

Expand Down Expand Up @@ -309,6 +345,7 @@ impl RustType<ProtoComputeLog> for ComputeLog {
}
}

/// TODO(#25239): Add documentation.
pub static DEFAULT_LOG_VARIANTS: Lazy<Vec<LogVariant>> = Lazy::new(|| {
let default_logs = vec![
LogVariant::Timely(TimelyLog::Operates),
Expand Down Expand Up @@ -350,6 +387,7 @@ impl LogVariant {
.unwrap_or_else(|| (0..arity).collect())
}

/// TODO(#25239): Add documentation.
pub fn desc(&self) -> RelationDesc {
match self {
LogVariant::Timely(TimelyLog::Operates) => RelationDesc::empty()
Expand Down
Loading

0 comments on commit f7cd1fe

Please sign in to comment.