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

chore(weave): Further split out Query/Trace #2182

Closed
wants to merge 117 commits into from

Conversation

andrewtruong
Copy link
Collaborator

@andrewtruong andrewtruong commented Aug 21, 2024

Summary

Do not merge until after weave 0.50.15 release!

Corresponding change in core: https://github.com/wandb/core/pull/23467

  1. This PR moves a lot of code and reorganizes the repo in prep to eject the query and trace server code.
    1. Legacy code is moved to weave/legacy/
    2. Trace code is kept in the top-level weave/
    3. Splits out the legacy W&B api dependency into its own submodule wandb_api
    4. Dependencies of trace on query are minimized -- what remains are mostly in tests, or relate to storage, urls, context_state, artifact_fs, and some stuff for fastapi
    5. Tests and CI are broken out for trace and query.
      1. Tests are moved to to either weave/tests/ (for trace), or weave/legacy/tests/ (for legacy)
      2. mypy.ini is split into trace and legacy
      3. pre-commit hooks are updated for trace and legacy
    6. Scripts, docs, misc files are consolidated where it makes sense and updated for readability
    7. Top-level requirements now reflect just the trace code -- legacy requirements moved to weave/legacy/requirements.* -- Deferred to simplify this PR
    8. mypy.ini broken out for trace and legacy respectively

Review suggestions

  1. This change seems large, but is mostly file moving and updating paths. The best way to review is probably to view the directory and see how files have shifted
  2. This is similar to the previous legacy refactor and will require a force merge to resolve the dual-dependency in core.

Missing pieces (too hard atm):

  1. conftest.py
  2. weave_server.py
  3. weave_server.sh
  4. weave-js/
  5. frontend/
  6. trace_server/
  7. integration_test/
  8. Fully eject all deps from trace (and some from query)

@andrewtruong andrewtruong changed the base branch from master to andrew/tests-legacy2 August 21, 2024 00:55
@circle-job-mirror
Copy link

circle-job-mirror bot commented Aug 21, 2024

@andrewtruong andrewtruong force-pushed the andrew/more-legacy-split branch 16 times, most recently from 25d0be7 to 15af662 Compare August 21, 2024 15:55
Base automatically changed from andrew/tests-legacy2 to master August 21, 2024 17:38
@andrewtruong andrewtruong force-pushed the andrew/more-legacy-split branch 11 times, most recently from f9b17cf to a91d113 Compare August 22, 2024 04:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant