Skip to content

Drop editor in favor of ord-app - #172

Merged
skearnes merged 5 commits into
mainfrom
drop-editor
Aug 4, 2026
Merged

Drop editor in favor of ord-app#172
skearnes merged 5 commits into
mainfrom
drop-editor

Conversation

@skearnes

@skearnes skearnes commented May 4, 2026

Copy link
Copy Markdown
Member

Summary

Removes the legacy Flask editor under ord_interface/editor/. Its job belongs to the standalone ord-app now, and the Contribute link in the header already points there.

Everything that existed only to serve the editor goes with it:

  • ord_interface/editor/ and its Flask entrypoint ord_interface/interface.py.
  • The flask gunicorn worker in start_app.sh, and the flask upstream + /editor/ proxy in nginx.conf.
  • The closure-library / protobuf-js / protoc / closure-compiler / jquery-externs build chain in the Dockerfile, along with the default-jre install and the ord-schema v0.3.93 source download that fed it — and the ARCH build arg, which existed only to pick a protoc binary.
  • The editor schema + migrate.py step in build_test_database.sh, and node editor/js/test.js in run_tests.sh.
  • In CI: --ignore=ord_interface/editor, the npm install puppeteer step, and the setup-node step that served it. test_app moves from ubuntu-22.04 back to ubuntu-latest, since the pin was an AppArmor/puppeteer workaround.
  • The flask and pygithub dependencies, and the ruff/ty excludes that kept the editor and its entrypoint out of the linters.
  • The GH_CLIENT_ID/GH_CLIENT_SECRET passthrough in docker-compose.yml.

nginx 301s /editor and /editor/* to https://app.open-reaction-database.org/, so old bookmarks land on the new editor rather than a blank SPA shell.

Drive-by cleanups

  • setup_test_postgres() normalizes its connection URL to the psycopg (v3) driver. SQLAlchemy was otherwise reaching for psycopg2 on the bare postgresql:// URL that testing.postgresql produces — the last psycopg2 caller in the repo — so psycopg2-binary comes out of the dependency list and main.py no longer has to patch the driver into the URL itself.
  • Drops the *_pb2.py ruff/ty excludes: no generated protobuf wrappers are checked in or built here (the editor generated JS protos, not Python).

Retirement timeline

@bdeadman asked (below) that we not pull the editor out of production without giving the user community a clear deadline to retrieve their data. That happened in #178: since 2026-05-14 the editor's login and datasets pages have carried a banner announcing deactivation after July 31, 2026, pointing at the new editor and the announcement post, and giving help@open-reaction-database.org as a contact. That date has passed.

Deployment

The AWS Copilot manifest that pinned the front-end healthcheck to /editor/healthcheck is already gone from main (#174), and the Pulumi interface stack's target group uses the ALB default health check path (/), which the SPA serves. Nothing to change on the deployment side.

Follow-ups (out of scope for this PR)

  • Delete the unused SSM parameters under /copilot/ord-editor/secrets/ (GH_CLIENT_ID, GH_CLIENT_SECRET) and the GitHub OAuth app behind them.
  • Retire the editor database in ord-infrastructure (stacks/database/__main__.py) once the editor is off production.

Test plan

All of the below ran against d18ec48, the current head. CI is green on that commit as well: check_python, check_licenses, check_javascript, all 8 test_ord_interface jobs, and all 4 test_app jobs (which run ./run_tests.sh end to end).

  • uv run pytest — 91 passed
  • ruff check, ruff format --check, and ty check clean on ord_interface
  • ./run_tests.sh — test database builds without the editor schema/migrate step, the slimmed-down image builds, and the app tests pass (75 passed)
  • Container starts with only /run/fastapi.sock (no flask socket); gunicorn runs ord_interface.api.main:app alone, and the image has no ord_interface/editor/
  • /, /browse, /search, /about, and /api/datasets return 200 through nginx; /ketcher takes nginx's directory redirect to /ketcher/ and then 200
  • /editor, /editor/, /editor/foo, /editor/healthcheck all 301 to https://app.open-reaction-database.org/; /editorx falls through to the SPA

🤖 Generated with Claude Code

Greptile Summary

The PR removes the retired Flask editor and its build, runtime, test, and dependency infrastructure in favor of the standalone ORD app.

  • Removes the editor implementation, Flask entrypoint, worker, database setup, and obsolete frontend content.
  • Simplifies the Docker image, CI workflow, dependency lock, and startup scripts.
  • Redirects legacy /editor URLs to the standalone application and updates deployment configuration accordingly.
  • Normalizes test-database SQLAlchemy URLs to use the installed psycopg v3 driver.

Confidence Score: 5/5

The PR appears safe to merge, with the editor removal consistently reflected across routing, packaging, startup, tests, and deployment configuration.

No concrete changed-code failure remains after checking the retained database initialization and routing paths; the removed editor infrastructure is replaced by the documented external redirect.

Important Files Changed

Filename Overview
ord_interface/nginx.conf Removes the Flask upstream and redirects legacy editor paths while preserving SPA and FastAPI routing.
ord_interface/Dockerfile Removes editor-only build dependencies and artifacts while retaining the SPA, Ketcher, and FastAPI image build.
ord_interface/start_app.sh Removes the Flask gunicorn process so the container runs only nginx and FastAPI.
ord_interface/api/testing.py Forces SQLAlchemy test connections onto the installed psycopg v3 driver without changing database semantics.
pyproject.toml Removes editor-only Flask, GitHub, and psycopg2 dependencies and obsolete tool exclusions.
uv.lock Refreshes the lockfile to remove dependency packages no longer required after deleting the editor.
.github/workflows/tests.yml Removes editor-specific test setup and permits the application job to use the current Ubuntu runner.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  User[Browser request] --> Nginx
  Nginx -->|/editor or /editor/*| ORDApp[Standalone ORD app]
  Nginx -->|/api/*| FastAPI
  Nginx -->|Other paths| SPA[React SPA]
  FastAPI --> PostgreSQL
Loading

Reviews (1): Last reviewed commit: "Point the test database at psycopg and d..." | Re-trigger Greptile

The Flask-based editor under ord_interface/editor/ has been replaced by
the standalone ord-app (https://app.open-reaction-database.org). This
removes the editor and everything that existed only to support it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@skearnes
skearnes requested review from bdeadman and connorcoley May 4, 2026 02:29
skearnes and others added 2 commits May 3, 2026 22:33
- nginx: 301 /editor and /editor/* to https://app.open-reaction-database.org/
  so existing bookmarks land on the new editor instead of a blank SPA shell.
- CI: bump test_app runner from ubuntu-22.04 to ubuntu-latest now that the
  puppeteer-incompatibility note no longer applies.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bdeadman

bdeadman commented May 5, 2026

Copy link
Copy Markdown
Collaborator

@skearnes - yes we should do this to clean up the code, but I hesitate to rip the old editor out of the production instance without communicating a clear timeline for the user community to remove any important data. The latest statement we have made on the old editor is from the announcement of the new editor here https://www.linkedin.com/pulse/next-generation-reaction-editor-here-open-reaction-database-tfste.

Comment thread ord_interface/about.html Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My understanding is that this 'about' file no longer needed since the website uses this file (https://github.com/open-reaction-database/ord-interface/blob/8731895b44a960f0448a72042fa9981df2846c22/app/src/views/About.vue) instead for the about page.

@bdeadman bdeadman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Happy to approve the code changes as they are, but I'd like to have a conversation about how we manage the retirement of the legacy editor from the production instance.

@skearnes
skearnes marked this pull request as draft May 12, 2026 22:47
skearnes added a commit that referenced this pull request May 12, 2026
- Drop the placeholder "How to Deploy" section.
- Fix the project layout: api/ is the FastAPI server (not /client),
  and the description was missing it entirely. Mark editor/ as
  legacy (being removed in #172).
- Tighten setup instructions and folder descriptions; trim filler.
- Flag the host-port 5432 collision for `docker compose up`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
skearnes added a commit that referenced this pull request May 13, 2026
* Remove stale config and accidental commits

- .pylintrc, .style.yapf: legacy linter/formatter configs replaced by
  ruff in #173.
- copilot/: AWS Copilot CLI manifest, replaced by Pulumi/ECS in the
  ord-infrastructure repository.
- dump.rdb: accidentally committed Redis snapshot.
- README.md, CONTRIBUTING.md: setup.py / pip install instructions
  rewritten in terms of uv now that #173 has landed.
- .gitignore: add .pytest_cache/, .ruff_cache/, .venv/, dump.rdb so
  these stop showing up in future working trees.

Editor-tree `# pylint:` / `# pytype:` comments are intentionally left
alone; the editor is excluded from ruff/ty and is being removed in #172.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Refresh README

- Drop the placeholder "How to Deploy" section.
- Fix the project layout: api/ is the FastAPI server (not /client),
  and the description was missing it entirely. Mark editor/ as
  legacy (being removed in #172).
- Tighten setup instructions and folder descriptions; trim filler.
- Flag the host-port 5432 collision for `docker compose up`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Apply review feedback on cleanup PR

- CONTRIBUTING.md: sweep remaining `ord-schema` references to
  `ord-interface` (title, issue-tracker URLs, fork instructions). The
  goals/non-goals readthedocs link is left as-is and rephrased to
  point at "the broader Open Reaction Database" so it's clear the
  reference is intentional.
- README.md: add a brief Deployment section pointing at the
  ord-infrastructure repo (Pulumi/ECS) now that the old "How to
  Deploy" placeholder is gone.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* .gitignore: add trailing slashes to directory-only patterns

Make the pre-existing directory patterns consistent with the new ones
added in this branch (`.pytest_cache/`, `.ruff_cache/`, `.venv/`) and
the already-slashed ones above (`.idea/`, `docs/_build/`, `.vscode/`).

Trailing slash restricts a pattern to match directories only, which
is what we actually want for `__pycache__`, `*.egg-info`,
`.ipynb_checkpoints`, `build`, `dist`, `ketcher`, `standalone`,
`node_modules`, and `coverage`. A future file accidentally named one
of these wouldn't be silently ignored.

`**/.pnp` is left as-is because Yarn 2+ uses `.pnp.cjs` files while
Yarn 1 used a `.pnp` directory; the un-slashed form matches both.

Verified `git check-ignore` still resolves the existing
`.venv/`, `ord_interface.egg-info/`, `ord_interface/__pycache__/`
directories through the new rules.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
skearnes and others added 2 commits August 3, 2026 22:02
Reapply the editor removal on top of everything main picked up since May:
the React/Vite migration, the uv toolchain, the Copilot config cleanup,
and the removal of about.html and dump.rdb (all of which this branch had
done its own way against the Vue/setup.py layout).

- Dockerfile: drop the closure-library/protoc/closure-compiler/jquery-externs
  layers, the ord-schema source download they fed, default-jre, and the editor
  COPY/build steps; keep the Ketcher fetch and the Vite build.
- nginx.conf: drop the flask upstream and /editor/ proxy; 301 /editor and
  /editor/* to https://app.open-reaction-database.org/.
- tests.yml: drop --ignore=ord_interface/editor, the puppeteer install and the
  setup-node step that existed only for the editor JS tests, and move test_app
  back to ubuntu-latest.
- pyproject.toml: drop flask and pygithub, and the ruff/ty excludes that kept
  the editor and its entrypoint out of the linters.
- README: drop the editor from the project layout and the Apple-silicon
  ARCH build-arg note, which went away with protoc.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
setup_test_postgres() normalizes its URL to the psycopg (v3) driver, so the
bare postgresql:// URL testing.postgresql produces no longer sends SQLAlchemy
looking for psycopg2. That was the last psycopg2 caller, so psycopg2-binary
comes out of the dependency list, and main.py no longer has to patch the
driver into the URL before handing it over.

The ruff and ty excludes for *_pb2.py also go: no generated protobuf wrappers
are checked in or built here -- the editor generated JS protos, not Python.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@skearnes
skearnes marked this pull request as ready for review August 4, 2026 02:08
@skearnes
skearnes requested a review from bdeadman August 4, 2026 02:08
@skearnes

skearnes commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

@bdeadman it's August 😄

@skearnes
skearnes merged commit d5e6981 into main Aug 4, 2026
17 checks passed
@skearnes
skearnes deleted the drop-editor branch August 4, 2026 23:39
skearnes added a commit to open-reaction-database/ord-infrastructure that referenced this pull request Aug 5, 2026
The Flask editor was the only consumer of GH_CLIENT_ID/GH_CLIENT_SECRET, and
it is gone from ord-interface as of open-reaction-database/ord-interface#172.
The task definition no longer needs them, the execution role no longer needs
read access to the secret, and the `github-client` secret itself has no
remaining reader.

Applied to prod already: the running task exposes only POSTGRES_PASSWORD and
ANTHROPIC_API_KEY, and the secret is in its 30-day recovery window.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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