Skip to content

Qarnot cloud interface #13

Open
basile-simvia wants to merge 5 commits into
mainfrom
feat/run_remote_qarnot
Open

Qarnot cloud interface #13
basile-simvia wants to merge 5 commits into
mainfrom
feat/run_remote_qarnot

Conversation

@basile-simvia
Copy link
Copy Markdown
Collaborator

Add the possibility to submit code_aster computation to qarnot
Various modifications in CI/CD

…ckaging

- Replace archived actions/create-release@v1 and actions/upload-release-asset@v1
  with softprops/action-gh-release@v2 (single step for tag + assets)
- Add pre-release support via SemVer-suffixed tags (-rc.N, -beta.N, -alpha.N):
  auto-detected from the tag, propagated to prerelease/make_latest flags
- Gate release on unitest + e2e jobs via workflow_call (release.yml now
  orchestrates build + unitest + e2e + release with explicit needs)
- Make unitest.yml callable from other workflows (workflow_call trigger)
- Drop continue-on-error on .deb and .rpm builds so packaging failures
  surface immediately instead of silently producing incomplete releases
- Declare explicit contents: write permission on release workflow
- Bump actions/checkout to v5 in doc.yml and release.yml (was v4)
- Replace unmaintained actions-rs/toolchain@v1 with dtolnay/rust-toolchain@stable
  in doc.yml and unitest.yml
- Declare explicit contents: read permissions on build.yml, unitest.yml,
  e2e.yml, pr.yml (defense-in-depth, doc.yml already had its block)
- Remove 'edited' from pr.yml trigger types to avoid rebuilds on PR
  title/description edits
- Add reusable composite action .github/actions/setup-rust that installs
  apt build deps and a stable Rust toolchain, with optional cross-compile
  target and a sudo toggle for in-container usage
- Refactor build.yml, doc.yml, and unittest.yml to use the composite,
  removing duplicated install steps
- Rename unitest.yml to unittest.yml (typo fix); update job names and the
  workflow_call reference in release.yml accordingly
- Add workflow_dispatch trigger to build.yml so the build can be
  launched manually from the Actions UI on any branch
- Default artifact-name prefix is 'manual-' (overridable from the UI)
  to avoid collision with automatic build artifacts
- Make the artifact-name expression null-safe (|| '') to handle the
  three trigger contexts uniformly (push/workflow_dispatch/workflow_call)
Copy link
Copy Markdown
Contributor

@SIMVIA-lucas-sovre SIMVIA-lucas-sovre left a comment

Choose a reason for hiding this comment

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

Vu mon peu de compétences en RUST c'est complexe pour moi de review la code quality, mais j'ai trouvé quelques petites corréctions à apporter sur le fonctionnement en lui même.

Bonne journée !

Comment thread src/main.rs
Comment on lines +119 to +126
QarnotAction::Show => qarnot_config_show(),
QarnotAction::List => qarnot_list_jobs(),
QarnotAction::Status { uuid } => qarnot_status(&uuid),
QarnotAction::Download { uuid, output } => qarnot_download(&uuid, output),
QarnotAction::Stdout { uuid } => qarnot_stdout(&uuid),
QarnotAction::Stderr { uuid } => qarnot_stderr(&uuid),
QarnotAction::Abort { uuid } => qarnot_abort(&uuid),
QarnotAction::Delete { uuid, purge } => qarnot_delete(&uuid, purge),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pour moi, si l'utilisateur n'as pas opt-out de la télémetrie il faudrait que chacune de ces commandes nous soient remontées (statistique d'usage uniquement) sinon on ne sauras pas si ça fonctionne chez l'utilisateur final, ou si c'est utilisé tout court.

A voir avec @ulysse-bouchet-simvia pour intégrer la possibilité de remonter le nom de la cmd appelée

Comment thread src/qarnot/config.rs
Comment on lines +202 to +220
/// Write a [`PartialQarnotConfig`] to disk as pretty-printed JSON.
pub fn write_partial_config(path: &Path, config: &PartialQarnotConfig) -> Result<(), QarnotError> {
let content = serde_json::to_string_pretty(config)
.map_err(|e| QarnotError::ConfigError(format!("Serialization failed: {}", e)))?;
std::fs::write(path, content)
.map_err(|e| QarnotError::ConfigError(format!("Cannot write {}: {}", path.display(), e)))?;
Ok(())
}

/// Write a fully resolved [`QarnotConfig`] to disk (used when running
/// `cave qarnot config` to produce a global file).
#[allow(dead_code)]
pub fn write_qarnot_config(path: &Path, config: &QarnotConfig) -> Result<(), QarnotError> {
let content = serde_json::to_string_pretty(config)
.map_err(|e| QarnotError::ConfigError(format!("Serialization failed: {}", e)))?;
std::fs::write(path, content)
.map_err(|e| QarnotError::ConfigError(format!("Cannot write {}: {}", path.display(), e)))?;
Ok(())
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

le token api est écrit en clair dans un fichier local, il faudrait au moins stdout à l'utilisateur que le token est en clair dans le fichier /chemin/vers/fichierconf

Et lui dire que ce fichier devrais être ajouté au gitignore si un on detecte être dans un repos git

Comment thread src/qarnot_commands.rs
Comment on lines +295 to +301
fn truncate(s: &str, max: usize) -> String {
if s.len() <= max {
s.to_string()
} else {
format!("{}…", &s[..max - 1])
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

je ne suis pas compétent en rust mais d'aprés ce que j'ai pu analyser avec claude code

truncate peut paniquer sur une entrée UTF-8 (ligne 299)

&s[..max - 1] indexe des octets, pas des caractères. Comme s vaut ici job.name (issu de --name, non contraint), un nom contenant un accent ou un emoji et dépassant
35 octets sera coupé sur une frontière multi-octets → panic au runtime, et cave qarnot list plante.

Accessoirement, le garde s.len() <= max compare aussi des longueurs en octets : une chaîne visuellement courte mais riche en UTF-8 sera tronquée inutilement.

Suggestion (raisonner en char) :

Suggested change
fn truncate(s: &str, max: usize) -> String {
if s.len() <= max {
s.to_string()
} else {
format!("{}…", &s[..max - 1])
}
}
fn truncate(s: &str, max: usize) -> String {
if s.chars().count() <= max {
s.to_string()
} else {
let head: String = s.chars().take(max - 1).collect();
format!("{}…", head)
}
}

Comment thread src/qarnot/bucket.rs
Comment on lines +221 to +224
let bytes = resp.body.collect().await.map_err(|e| {
QarnotError::S3Error(format!("Read body of '{}': {}", key, e))
})?;
std::fs::write(&local_path, bytes.into_bytes()).map_err(|e| {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

resp.body.collect().await bufferise l'objet complet en mémoire, puis into_bytes() en fait une copie avant l'écriture.

code_aster produit des .med/.resu qui peuvent peser plusieurs Go → un seul gros résultat peut faire OOM le process. Il faut streamer le corps de la réponse directement vers le fichier au lieu de tout accumuler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants