From 46a1cff6f7c1d454b9762264ca34be0362761881 Mon Sep 17 00:00:00 2001 From: Kostia Balytskyi Date: Wed, 24 Feb 2021 07:43:20 -0800 Subject: [PATCH] scs: add stub implementation of megarepo svc Summary: This diff adds megarepo service methods to SCS. Each of the methods just returns "not implemented" at the moment. The main goal of this diff is to provide a reference against which a sample client can be built. This diffs makes two decisions that came from the source control discussion presentation: 1. expensive calls are asynchronous. This means that they are separated into an initial "request placing" call, which returns a polling token; and a polling call, which takes said token and blocks until it either times out or until the request is satisfied 1. racy calls take target location and only advance target if this location does not change until the advancement time comes. This ensures that the clients are aware that changing sync config versions or re-merging sources should be done why `megarepo_sync_changeset` calls are paused There are also some decisions that I made while writing this, which may be questionable: 1. The polling token types are just string aliases. This is fine atm, but may be a problem in the future if we want to implement `AddScubaResponse` or `AddScubaParams` for some but not others. Moreover, now I had to implement both traits for `String`, which I am also not too happy about. Maybe I should turn polling tokens into wrapper structs right now 1. all new methods start with `megarepo_` or `poll_megarepo`. Generally I dislike name-based namespacing, but in this case the alternative is to either keep the names like `sync_changeset` (not descriptive enough), or move these methods into a separate service definition (too much overhead, and probably not desirable from operational perspective anyways). So I went with prefixes. 1. **the most questionable decision IMO** is about a config design. ATM the `SyncTargetConfig` type contains the `Target` internally. At the same time, `MegarepoChangeTargetConfigParams` contains both the `Target` and the `SyncTargetConfig` and will error out if `SyncTargetConfig` refers to a different target. The API could be made simpler by just taking `SyncTargetConfig`, but it feels a bit wrong to not have the client give us the target they want to change the config for. I can be easily persuaded to remove `Target` from `MegarepoChangeTargetConfigParams`. It can also be done incrementally, until we have any real clients, so figuring this out now is not important. Reviewed By: markbt Differential Revision: D26549580 fbshipit-source-id: 2d38d6273fb5ec46b846a242f4f1abf61502cb1c --- eden/mononoke/scs_server/src/errors.rs | 15 +++ .../scs_server/src/methods/megarepo.rs | 94 +++++++++++++++++++ eden/mononoke/scs_server/src/methods/mod.rs | 1 + eden/mononoke/scs_server/src/scuba_params.rs | 13 +++ .../mononoke/scs_server/src/scuba_response.rs | 15 +++ .../scs_server/src/source_control_impl.rs | 32 +++++++ 6 files changed, 170 insertions(+) create mode 100644 eden/mononoke/scs_server/src/methods/megarepo.rs diff --git a/eden/mononoke/scs_server/src/errors.rs b/eden/mononoke/scs_server/src/errors.rs index 644a481cc3..540434ea6b 100644 --- a/eden/mononoke/scs_server/src/errors.rs +++ b/eden/mononoke/scs_server/src/errors.rs @@ -127,6 +127,14 @@ impl_into_thrift_error!(service::FileContentChunkExn); impl_into_thrift_error!(service::FileDiffExn); impl_into_thrift_error!(service::CommitLookupXrepoExn); impl_into_thrift_error!(service::RepoListHgManifestExn); +impl_into_thrift_error!(service::MegarepoAddSyncTargetConfigExn); +impl_into_thrift_error!(service::MegarepoAddSyncTargetExn); +impl_into_thrift_error!(service::MegarepoChangeTargetConfigExn); +impl_into_thrift_error!(service::MegarepoChangeTargetConfigPollExn); +impl_into_thrift_error!(service::MegarepoSyncChangesetExn); +impl_into_thrift_error!(service::MegarepoSyncChangesetPollExn); +impl_into_thrift_error!(service::MegarepoRemergeSourceExn); +impl_into_thrift_error!(service::MegarepoRemergeSourcePollExn); pub(crate) fn invalid_request(reason: impl ToString) -> thrift::RequestError { thrift::RequestError { @@ -209,3 +217,10 @@ pub(crate) fn not_available(reason: String) -> thrift::RequestError { reason, } } + +pub(crate) fn not_implemented(reason: impl ToString) -> thrift::RequestError { + thrift::RequestError { + kind: thrift::RequestErrorKind::NOT_IMPLEMENTED, + reason: reason.to_string(), + } +} diff --git a/eden/mononoke/scs_server/src/methods/megarepo.rs b/eden/mononoke/scs_server/src/methods/megarepo.rs new file mode 100644 index 0000000000..0cae317f94 --- /dev/null +++ b/eden/mononoke/scs_server/src/methods/megarepo.rs @@ -0,0 +1,94 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This software may be used and distributed according to the terms of the + * GNU General Public License version 2. + */ + +use context::CoreContext; +use source_control as thrift; + +use crate::errors; +use crate::source_control_impl::SourceControlServiceImpl; + +impl SourceControlServiceImpl { + pub(crate) async fn megarepo_add_sync_target_config( + &self, + _ctx: CoreContext, + _params: thrift::MegarepoAddConfigParams, + ) -> Result { + Err(errors::ServiceError::from(errors::not_implemented( + "megarepo_add_sync_target_config is not yet implemented", + ))) + } + + pub(crate) async fn megarepo_add_sync_target( + &self, + _ctx: CoreContext, + _params: thrift::MegarepoAddTargetParams, + ) -> Result { + Err(errors::ServiceError::from(errors::not_implemented( + "megarepo_add_sync_target is not yet implemented", + ))) + } + + pub(crate) async fn megarepo_change_target_config( + &self, + _ctx: CoreContext, + _params: thrift::MegarepoChangeTargetConfigParams, + ) -> Result { + Err(errors::ServiceError::from(errors::not_implemented( + "megarepo_change_target_config is not yet implemented", + ))) + } + + pub(crate) async fn megarepo_change_target_config_poll( + &self, + _ctx: CoreContext, + _params: thrift::MegarepoChangeConfigToken, + ) -> Result { + Err(errors::ServiceError::from(errors::not_implemented( + "poll_megarepo_change_config is not yet implemented", + ))) + } + + pub(crate) async fn megarepo_sync_changeset( + &self, + _ctx: CoreContext, + _params: thrift::MegarepoSyncChangesetParams, + ) -> Result { + Err(errors::ServiceError::from(errors::not_implemented( + "megarepo_sync_changeset is not yet implemented", + ))) + } + + pub(crate) async fn megarepo_sync_changeset_poll( + &self, + _ctx: CoreContext, + _params: thrift::MegarepoSyncChangesetToken, + ) -> Result { + Err(errors::ServiceError::from(errors::not_implemented( + "poll_megarepo_sync_changeset is not yet implemented", + ))) + } + + pub(crate) async fn megarepo_remerge_source( + &self, + _ctx: CoreContext, + _params: thrift::MegarepoRemergeSourceParams, + ) -> Result { + Err(errors::ServiceError::from(errors::not_implemented( + "megarepo_remerge_source is not yet implemented", + ))) + } + + pub(crate) async fn megarepo_remerge_source_poll( + &self, + _ctx: CoreContext, + _params: thrift::MegarepoRemergeSourceToken, + ) -> Result { + Err(errors::ServiceError::from(errors::not_implemented( + "poll_megarepo_remerge_source is not yet implemented", + ))) + } +} diff --git a/eden/mononoke/scs_server/src/methods/mod.rs b/eden/mononoke/scs_server/src/methods/mod.rs index 300cc93607..d28d691c3e 100644 --- a/eden/mononoke/scs_server/src/methods/mod.rs +++ b/eden/mononoke/scs_server/src/methods/mod.rs @@ -14,6 +14,7 @@ use crate::source_control_impl::SourceControlServiceImpl; pub(crate) mod commit; pub(crate) mod commit_path; pub(crate) mod file; +pub(crate) mod megarepo; pub(crate) mod repo; pub(crate) mod tree; diff --git a/eden/mononoke/scs_server/src/scuba_params.rs b/eden/mononoke/scs_server/src/scuba_params.rs index 5be6287338..93be0215a7 100644 --- a/eden/mononoke/scs_server/src/scuba_params.rs +++ b/eden/mononoke/scs_server/src/scuba_params.rs @@ -303,3 +303,16 @@ impl AddScubaParams for thrift::RepoListHgManifestParams { ); } } + +// Various Megarepo polling tokens are just aliases for String +impl AddScubaParams for String {} + +impl AddScubaParams for thrift::MegarepoSyncChangesetParams {} + +impl AddScubaParams for thrift::MegarepoRemergeSourceParams {} + +impl AddScubaParams for thrift::MegarepoChangeTargetConfigParams {} + +impl AddScubaParams for thrift::MegarepoAddTargetParams {} + +impl AddScubaParams for thrift::MegarepoAddConfigParams {} diff --git a/eden/mononoke/scs_server/src/scuba_response.rs b/eden/mononoke/scs_server/src/scuba_response.rs index 9f0f70ae5f..5ce2bccd06 100644 --- a/eden/mononoke/scs_server/src/scuba_response.rs +++ b/eden/mononoke/scs_server/src/scuba_response.rs @@ -74,3 +74,18 @@ impl AddScubaResponse for thrift::FileDiffResponse {} impl AddScubaResponse for thrift::TreeListResponse {} impl AddScubaResponse for thrift::RepoListHgManifestResponse {} + +// MegarepoRemergeSourceToken, MegarepoChangeConfigToken +// MegarepoSyncChangesetToken are all just aliases for String +impl AddScubaResponse for String {} + +// TODO: report cs_ids where possible +impl AddScubaResponse for thrift::MegarepoRemergeSourceResponse {} + +impl AddScubaResponse for thrift::MegarepoSyncChangesetResponse {} + +impl AddScubaResponse for thrift::MegarepoChangeTargetConfigResponse {} + +impl AddScubaResponse for thrift::MegarepoAddTargetResponse {} + +impl AddScubaResponse for thrift::MegarepoAddConfigResponse {} diff --git a/eden/mononoke/scs_server/src/source_control_impl.rs b/eden/mononoke/scs_server/src/source_control_impl.rs index b3f9523a6a..00f32f1d0b 100644 --- a/eden/mononoke/scs_server/src/source_control_impl.rs +++ b/eden/mononoke/scs_server/src/source_control_impl.rs @@ -574,6 +574,38 @@ impl SourceControlService for SourceControlServiceThriftImpl { params: thrift::RepoLandStackParams, ) -> Result; + async fn megarepo_add_sync_target_config( + params: thrift::MegarepoAddConfigParams, + ) -> Result; + + async fn megarepo_add_sync_target( + params: thrift::MegarepoAddTargetParams, + ) -> Result; + + async fn megarepo_change_target_config( + params: thrift::MegarepoChangeTargetConfigParams, + ) -> Result; + + async fn megarepo_change_target_config_poll( + token: thrift::MegarepoChangeConfigToken, + ) -> Result; + + async fn megarepo_sync_changeset( + params: thrift::MegarepoSyncChangesetParams, + ) -> Result; + + async fn megarepo_sync_changeset_poll( + token: thrift::MegarepoSyncChangesetToken, + ) -> Result; + + async fn megarepo_remerge_source( + params: thrift::MegarepoRemergeSourceParams, + ) -> Result; + + async fn megarepo_remerge_source_poll( + token: thrift::MegarepoRemergeSourceToken, + ) -> Result; + async fn repo_list_hg_manifest( repo: thrift::RepoSpecifier, params: thrift::RepoListHgManifestParams,