Skip to content

Conversation

@my-vegetable-has-exploded
Copy link
Contributor

@my-vegetable-has-exploded my-vegetable-has-exploded commented Dec 22, 2025

Why are these changes needed?

Move historyserver/pkg/collector/logcollector/storage to historyserver/pkg/storage and updata imports.

Related issue number

Closes #4278

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

Signed-off-by: my-vegetable-has-exploded <[email protected]>
Signed-off-by: my-vegetable-has-exploded <[email protected]>
Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

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

cc @lorriexingfang to take a look!

Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

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

cc @JiangJiaWei1103 @machichima need you to help review.

@Future-Outlier Future-Outlier moved this from In Progress to Todo in @Future-Outlier's kuberay project Jan 2, 2026
@machichima
Copy link
Collaborator

Could you also help updating the README?

  • There's no backend/ now, we use pkg/
  • Please add a bullet point for pkg/storage

Thanks!

### Code Structure
- `cmd/`: Main applications (collector and historyserver)
- `backend/`: Core logic for storage backends and collection
- `backend/collector/`: Collector-specific code
- `backend/historyserver/`: History server implementation
- `dashboard/`: Web UI files

Copy link
Contributor

@JiangJiaWei1103 JiangJiaWei1103 left a comment

Choose a reason for hiding this comment

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

Would you mind merging the master branch so the collector e2e tests can be triggered? Thanks.

Local e2e tests pass as below:

Screenshot 2026-01-03 at 9 27 45 PM

@my-vegetable-has-exploded
Copy link
Contributor Author

Could you also help updating the README?

* There's no `backend/` now, we use `pkg/`

* Please add a bullet point for `pkg/storage`

Thanks!

### Code Structure
- `cmd/`: Main applications (collector and historyserver)
- `backend/`: Core logic for storage backends and collection
- `backend/collector/`: Collector-specific code
- `backend/historyserver/`: History server implementation
- `dashboard/`: Web UI files

done.

Copy link
Contributor

@JiangJiaWei1103 JiangJiaWei1103 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Comment on lines +121 to +123
- `pkg/`: Core logic for storage backends and collection
- `pkg/collector/`: Collector-specific code
- `pkg/storage/`: Storage backend implementations
Copy link
Contributor

@JiangJiaWei1103 JiangJiaWei1103 Jan 4, 2026

Choose a reason for hiding this comment

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

Great! The modification is within the scope of this PR.

Just a quick reminder: this documentation doesn’t reflect the current history server code structure (e.g., missing eventserver/). Instead of adding more components on top of outdated docs, I suggest opening a separate issue to update the documentation. Since the history server is still under active development, it may be better to postpone the doc update until we have a first stable version.

Issue link: #4338

cc @Future-Outlier

@Future-Outlier Future-Outlier moved this from Todo to In Progress in @Future-Outlier's kuberay project Jan 5, 2026
@CheyuWu CheyuWu self-requested a review January 6, 2026 16:23
Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

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

cursor review

@Future-Outlier
Copy link
Member

cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no bugs!

Copy link
Collaborator

@machichima machichima left a comment

Choose a reason for hiding this comment

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

LGTM!

@Future-Outlier Future-Outlier moved this from In Progress to can be merged in @Future-Outlier's kuberay project Jan 7, 2026
Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

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

thank you!

@rueian rueian merged commit fc6f162 into ray-project:master Jan 8, 2026
29 checks passed
@github-project-automation github-project-automation bot moved this from can be merged to Done in @Future-Outlier's kuberay project Jan 8, 2026
@my-vegetable-has-exploded
Copy link
Contributor Author

Thanks all.

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.

[history server][collector] Move storage's interface to correct order

8 participants