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

Add storage api for new http backend #488

Merged
merged 19 commits into from
Nov 1, 2023
Merged

Add storage api for new http backend #488

merged 19 commits into from
Nov 1, 2023

Conversation

al8n
Copy link
Contributor

@al8n al8n commented Oct 30, 2023

No description provided.

@al8n al8n added the enhancement New feature or request label Oct 30, 2023
@al8n al8n added this to the Nomos testnet (playground) milestone Oct 30, 2023
@al8n al8n self-assigned this Oct 30, 2023
@@ -29,6 +29,7 @@ pub struct AxumBackendSettings {
pub cl: OverwatchHandle,
pub carnot: OverwatchHandle,
pub network: OverwatchHandle,
pub storage: OverwatchHandle,
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we have multiple overwatch handle copies?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, I was wondering the same. But I have seen this is removed in a different PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, in the final PR, there is only one left.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be hard to not add them here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not hard, actually. I also considered removing copies in the initial PR (metrics-api), but there are some small refactors on trait design in the final PR, and the metrics-api one has already been reviewed. So, I chose to remove these handles in the final PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not clear what is the final PR. If you open PRs with a specific sequence in mind please put it in the title (e.g. API refactor [2/5] - storage) or description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the misleading. The final PR is #490. I will do this next time.

@al8n al8n requested a review from zeegomo October 30, 2023 15:27
Base automatically changed from da-blob-api to info-api November 1, 2023 07:25
@al8n al8n merged commit f3e01c3 into info-api Nov 1, 2023
6 checks passed
@al8n al8n deleted the storage-api-new branch November 1, 2023 07:25
al8n added a commit that referenced this pull request Nov 1, 2023
* Info api

* Da blob api (#487)

* Add storage api for new http backend (#488)

* Mempool add APIs (#489)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants