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

Clone app in core Dust #44

Merged
merged 2 commits into from
Nov 4, 2022
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
2 changes: 1 addition & 1 deletion core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,4 @@ bb8 = "0.8"
bb8-postgres = "0.8"
urlencoding = "2.1"
url = "2.3"
dns-lookup = "1.0"
dns-lookup = "1.0"
92 changes: 91 additions & 1 deletion core/bin/dust_api.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use anyhow::Result;
use anyhow::{anyhow, Result};
use axum::{
extract,
response::Json,
Expand Down Expand Up @@ -143,6 +143,95 @@ async fn projects_create(
}
}

/// Clones a project.
/// Simply consists in cloning the latest dataset versions, as we don't copy runs and hence specs.

async fn projects_clone(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might make more sense to call this function projects_fork and the endpoint /projects/<projectId>/fork. To me it's a clearer description of what's going on here and consistent with the UI.

extract::Extension(state): extract::Extension<Arc<APIState>>,
extract::Path(project_id): extract::Path<i64>,
) -> (StatusCode, Json<APIResponse>) {
let cloned = project::Project::new_from_id(project_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

The naming here was confusing for me. cloned implies that this is the cloned project (i.e. the new one), not the one that is being cloned (the parent). Maybe use a name like "parent" or "project_to_be_cloned"?


// Create cloned project
let project = match state.store.create_project().await {
Err(e) => {
return (
StatusCode::INTERNAL_SERVER_ERROR,
Json(APIResponse {
error: Some(APIError {
code: String::from("internal_server_error"),
message: format!("Failed to create cloned project: {}", e),
}),
response: None,
}),
)
}
Ok(project) => project,
};

// Retrieve datasets
let datasets = match state.store.list_datasets(&cloned).await {
Err(e) => {
return (
StatusCode::INTERNAL_SERVER_ERROR,
Json(APIResponse {
error: Some(APIError {
code: String::from("internal_server_error"),
message: format!("Failed to list cloned project datasets: {}", e),
}),
response: None,
}),
)
}
Ok(datasets) => datasets,
};

// Load and register datasets
let store = state.store.clone();
match futures::future::join_all(datasets.iter().map(|(d, v)| async {
let dataset = match store
.load_dataset(&cloned, &d.clone(), &v[0].clone().0)
.await?
{
Some(dataset) => dataset,
None => Err(anyhow!(
"Could not find latest version of dataset {}",
d.clone()
))?,
};
store.register_dataset(&project, &dataset).await?;
Ok::<(), anyhow::Error>(())
}))
.await
.into_iter()
.collect::<Result<Vec<_>>>()
{
Err(e) => {
return (
StatusCode::INTERNAL_SERVER_ERROR,
Json(APIResponse {
error: Some(APIError {
code: String::from("internal_server_error"),
message: format!("Failed to clone project datasets: {}", e),
}),
response: None,
}),
)
}
Ok(_) => (),
}

return (
StatusCode::OK,
Json(APIResponse {
error: None,
response: Some(json!({
"project": project,
})),
}),
);
}

/// Check a specification

#[derive(serde::Deserialize)]
Expand Down Expand Up @@ -669,6 +758,7 @@ async fn main() -> Result<()> {
.route("/", get(index))
// Projects
.route("/projects", post(projects_create))
.route("/projects/:project_id/clone", post(projects_clone))
Copy link
Contributor

Choose a reason for hiding this comment

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

See above comment on naming. I think that fork is probably a better name here. Thoughts?

// Specifications
.route(
"/projects/:project_id/specifications/check",
Expand Down
1 change: 0 additions & 1 deletion front/components/app/MainTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export default function MainTab({ app, current_tab, user, readOnly }) {
<Link href={tab.href} key={tab.name}>
<a
key={tab.name}
foo={tab.name}
className={classNames(
"whitespace-nowrap flex py-3 px-4 border-b-2 text-sm flex items-center",
tab.name == current_tab
Expand Down
5 changes: 3 additions & 2 deletions front/init/db.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { User, App, Dataset, Provider } from '../lib/models.js';
import { User, App, Dataset, Provider, Clone } from '../lib/models.js';

async function main() {
await User.sync({ alter: true });
await App.sync({ alter: true });
await Dataset.sync({ alter: true });
await Provider.sync({ alter: true });
await Clone.sync({ alter: true });
process.exit(0);
}

await main();
await main();
30 changes: 30 additions & 0 deletions front/lib/models.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,33 @@ export const Dataset = sequelize.define(
);
User.hasMany(Dataset);
App.hasMany(Dataset);

export const Clone = sequelize.define(
"clone",
{
id: {
type: DataTypes.INTEGER,
primaryKey: true,
autoIncrement: true,
},
fromId: {
type: DataTypes.INTEGER,
allowNull: false,
references: {
model: "apps",
key: "id",
},
},
toId: {
type: DataTypes.INTEGER,
allowNull: false,
references: {
model: "apps",
key: "id",
},
},
},
{}
);
Clone.belongsTo(App, { as: "from", foreignKey: "fromId" });
Clone.belongsTo(App, { as: "to", foreignKey: "toId" });
Loading