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

refactor: BlockAsDirReader and BlockAsDirWriter review and overhaul #319

Open
ata-nas opened this issue Oct 29, 2024 · 3 comments
Open

refactor: BlockAsDirReader and BlockAsDirWriter review and overhaul #319

ata-nas opened this issue Oct 29, 2024 · 3 comments
Labels
Improvement Code changes driven by non business requirements
Milestone

Comments

@ata-nas
Copy link
Contributor

ata-nas commented Oct 29, 2024

Story

AS A BN Developer
I WANT a review and overhaul if needed of the BlockAsDirReader and BlockAsDirWriter
SO THAT concerns about current implementation are addressed and we are assured of their proper functioning.

Tech Notes

Based on this comment and the thread that proceeds it, there are a few concerns with the current implementation of the BlockAsDirReader and BlockAsDirWriter, mainly:

  • currently both classes have a filed member filePerms which make no distinction between file and folder permissions
  • it is important that files and folders have a different set of permissions, so such distinction should be made if necessarry
  • it must be discussed if we should continue to use permissions like we do currently or another approach must be used, see comment
@ata-nas ata-nas added the Improvement Code changes driven by non business requirements label Oct 29, 2024
@ata-nas ata-nas added this to the 0.2.0 milestone Oct 29, 2024
@AlfredoG87
Copy link
Contributor

I don't think is worth it to continue to work on these implementations since we should remove them once the BlockAsFile Writer and Reader are completed.

@AlfredoG87 AlfredoG87 modified the milestones: 0.2.0, 0.3.0 Nov 8, 2024
@ata-nas
Copy link
Contributor Author

ata-nas commented Nov 11, 2024

I don't think is worth it to continue to work on these implementations since we should remove them once the BlockAsFile Writer and Reader are completed.

@AlfredoG87 during the implementation of #272 we reached the conclusion that we should revise these implementations here. It is true that if we shall delete them we should not work any longer over them, but my understanding is that the BlockAsDirReader and BlockAsDirWriter should NOT be deleted, @jsync-swirlds am I right about that?

@jsync-swirlds
Copy link
Member

I don't think is worth it to continue to work on these implementations since we should remove them once the BlockAsFile Writer and Reader are completed.

There is no good reason to remove these implementations, as they're still useful for testing, debugging, and development. Certainly they are not production implementations, but at most I could see moving them to a testFixtures module (and I'm not certain we need to do that).

The persistence service should have multiple reader and writer implementations both to allow for flexibility in deployment and to ensure we continue to write the system with that required flexibility in mind.
Example scenarios
What if I want to deploy a block node that only stores the last few hours of blocks, without compression?
What if I want a block node that stores the individual items on disk for another process to pick up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Code changes driven by non business requirements
Projects
None yet
Development

No branches or pull requests

3 participants