Skip to content

Conversation

@BenjaminCharmes
Copy link
Contributor

Closes #1332 & #1046

Implements a new API endpoint and background process to export full collections as .eln files

@codecov
Copy link

codecov bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 76.69903% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.95%. Comparing base (1a6d5e9) to head (27218d8).

Files with missing lines Patch % Lines
pydatalab/src/pydatalab/scheduler.py 44.11% 19 Missing ⚠️
pydatalab/src/pydatalab/routes/v0_1/export.py 76.05% 17 Missing ⚠️
pydatalab/src/pydatalab/export_utils.py 84.41% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1371      +/-   ##
==========================================
- Coverage   80.10%   79.95%   -0.15%     
==========================================
  Files          70       74       +4     
  Lines        4759     4965     +206     
==========================================
+ Hits         3812     3970     +158     
- Misses        947      995      +48     
Files with missing lines Coverage Δ
pydatalab/src/pydatalab/models/export_task.py 100.00% <100.00%> (ø)
pydatalab/src/pydatalab/mongo.py 81.81% <100.00%> (+0.99%) ⬆️
pydatalab/src/pydatalab/routes/v0_1/__init__.py 100.00% <100.00%> (ø)
pydatalab/src/pydatalab/export_utils.py 84.41% <84.41%> (ø)
pydatalab/src/pydatalab/routes/v0_1/export.py 76.05% <76.05%> (ø)
pydatalab/src/pydatalab/scheduler.py 44.11% <44.11%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cypress
Copy link

cypress bot commented Oct 6, 2025

datalab    Run #3981

Run Properties:  status check passed Passed #3981  •  git commit 18f786f28b ℹ️: Merge 27218d8c8bc8570969aa4bc3e979255b4d5ee5fc into 1a6d5e960c415f6c5e222fd99267...
Project datalab
Branch Review bc/export-ELN
Run status status check passed Passed #3981
Run duration 07m 35s
Commit git commit 18f786f28b ℹ️: Merge 27218d8c8bc8570969aa4bc3e979255b4d5ee5fc into 1a6d5e960c415f6c5e222fd99267...
Committer Ben Charmes
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 336
View all changes introduced in this branch ↗︎

@BenjaminCharmes BenjaminCharmes force-pushed the bc/export-ELN branch 5 times, most recently from 748ed5c to c85a1bf Compare October 7, 2025 14:30
@BenjaminCharmes BenjaminCharmes marked this pull request as ready for review October 7, 2025 14:38
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

This looks fantastic, thanks @BenjaminCharmes!

Just writing a couple of points here that we can discuss:

  • configuring amount of resources APScheduler is allowed to use
  • using pydantic models rather than direct database queries to create items etc.
  • a few bits of permissions we need to be careful with

Comment on lines 72 to 80
collection_exists = flask_mongo.db.collections.find_one({"collection_id": collection_id})
if not collection_exists:
return jsonify({"status": "error", "message": "Collection not found"}), 404

collection_with_perms = flask_mongo.db.collections.find_one(
{"collection_id": collection_id, **get_default_permissions(user_only=True)}
)
if not collection_with_perms:
return jsonify({"status": "error", "message": "Access denied"}), 403
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
collection_exists = flask_mongo.db.collections.find_one({"collection_id": collection_id})
if not collection_exists:
return jsonify({"status": "error", "message": "Collection not found"}), 404
collection_with_perms = flask_mongo.db.collections.find_one(
{"collection_id": collection_id, **get_default_permissions(user_only=True)}
)
if not collection_with_perms:
return jsonify({"status": "error", "message": "Access denied"}), 403
collection_with_perms = flask_mongo.db.collections.find_one(
{"collection_id": collection_id, **get_default_permissions(user_only=True)}
)
if not collection_with_perms:
return jsonify({"status": "error", "message": "Collection not found"}), 404

Better not to leak IDs that exist but the user does not have permission for, so just treat missing ID as 404

Copy link
Member

Choose a reason for hiding this comment

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

Can probably also set user_only=False

@ml-evs ml-evs self-assigned this Oct 17, 2025
source_path = Path(file_data["location"])
if source_path.exists():
dest_file = item_folder / file_data["name"]
shutil.copy2(source_path, dest_file)
Copy link
Member

Choose a reason for hiding this comment

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

Somehow the actual raw data files are not making it into the .eln file when I try to export them

Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Thanks @BenjaminCharmes -- will test locally then merge.

Potential next steps:

  1. adding the same button for 1 sample
  2. building on 1, "Export" button that opens a modal for a sample, where the user can export a graph of related samples, and potentially automatically create a collection for it

First attempt export as ELN
Use APScheduler

Use APScheduler

Use APScheduler

Use APScheduler

Use APScheduler

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

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

ELNFileFormat implementation roadmap

3 participants