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: Add all newer configuration items to Environment Variable mapping. #285

Closed
2 tasks
Tracked by #308
ata-nas opened this issue Oct 22, 2024 · 2 comments · Fixed by #405
Closed
2 tasks
Tracked by #308

chore: Add all newer configuration items to Environment Variable mapping. #285

ata-nas opened this issue Oct 22, 2024 · 2 comments · Fixed by #405
Assignees
Labels
Bug A error that causes the feature to behave differently than what was expected based on design docs P2 Required to be completed in the assigned milestone, but may or may not impact release schedule. Simulator Issue related to Block Stream Simulator
Milestone

Comments

@ata-nas
Copy link
Contributor

ata-nas commented Oct 22, 2024

Description

What happened

Currently, there is some missing support for some configurable values via environment variables.

What should to happen

In order to support environment variables for configuration classes, we need to map the conventional screaming snake case for environment variables (since still by default we are not able to get another case at runtime) to a convention the config api understands(e.g. MEDIATOR_RING_BUFFER_SIZE to mediator.ringBufferSize). This is done by updating the com.hedera.block.server.config.ServerMappedConfigSourceInitializer#MAPPINGS and we also need to update the respective awareness test com.hedera.block.server.config.ServerMappedConfigSourceInitializerTest#SUPPORTED_MAPPINGS. Also, alongside that, we should update the README.md to let the users know what are the correct screaming snake case keys for the configs.

Tasklist

  • Update all the other needed configurations that need to be supported to be set via environment variables in :server
  • Update all the other needed configurations that need to be supported to be set via environment variables in :simulator

Missing configuration that should be added:

  • From server:
    • producer.type → PRODUCER_TYPE (default: "PRODUCTION")
    • mediator.type → MEDIATOR_TYPE (default: "PRODUCTION")
  • From simulator:
    • From GrpcConfig.java:
      • grpc.serverAddress (default: "localhost")
      • grpc.port (default: 8080)
    • From BlockStreamConfig.java:
      • blockStream.simulatorMode (default: "PUBLISHER")
      • blockStream.delayBetweenBlockItems (default: 1_500_000)
      • blockStream.maxBlockItemsToStream (default: 100_000)
      • blockStream.streamingMode (default: "MILLIS_PER_BLOCK")
      • blockStream.millisecondsPerBlock (default: 1000)
      • blockStream.blockItemsBatchSize (default: 1000)
    • From BlockGeneratorConfig.java:
      • generator.generationMode (default: "DIR")
      • generator.folderRootPath (default: "")
      • generator.managerImplementation (default: "BlockAsFileBlockStreamManager")
      • generator.paddedLength (default: 36)
      • generator.fileExtension (default: ".blk.gz")
      • generator.startBlockNumber (default: 1)
      • generator.endBlockNumber (default: -1)

Version

0.2.0-SNAPSHOT

@ata-nas ata-nas added the Bug A error that causes the feature to behave differently than what was expected based on design docs label Oct 22, 2024
@ata-nas ata-nas added this to the 0.2.0 milestone Oct 22, 2024
@AlfredoG87 AlfredoG87 added Simulator Issue related to Block Stream Simulator P2 Required to be completed in the assigned milestone, but may or may not impact release schedule. labels Oct 22, 2024
@AlfredoG87
Copy link
Contributor

This is already merged, closing it.

@ata-nas
Copy link
Contributor Author

ata-nas commented Nov 11, 2024

This is already merged, closing it.

@AlfredoG87 no, this should not be closed as it is not implemented/merged anywhere.

In the above description, it is given example of an already existing config in order to showcase what must be done to all the others that need to be supported via environment variables. This issue IS NOT about the example above, but for all the rest.

I am reopening this. I also add a couple of checkboxes to make it more clear.

@ata-nas ata-nas reopened this Nov 11, 2024
@ata-nas ata-nas removed their assignment Nov 11, 2024
@AlfredoG87 AlfredoG87 modified the milestones: 0.2.0, 0.3.0 Nov 12, 2024
@jsync-swirlds jsync-swirlds changed the title bug: missing support for configuration through environment variables chore: Add all newer configuration items to Environment Variable mapping. Dec 11, 2024
@georgi-l95 georgi-l95 self-assigned this Dec 12, 2024
@georgi-l95 georgi-l95 linked a pull request Dec 13, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A error that causes the feature to behave differently than what was expected based on design docs P2 Required to be completed in the assigned milestone, but may or may not impact release schedule. Simulator Issue related to Block Stream Simulator
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants