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

feat: Embed genesis files. #2129

Closed
wants to merge 2 commits into from
Closed

feat: Embed genesis files. #2129

wants to merge 2 commits into from

Conversation

ajnavarro
Copy link
Contributor

@ajnavarro ajnavarro commented May 16, 2024

Proposal for embedding initial genesis data inside the gnoland binary instead of need them on the filesystem in the gnoroot folder.

It will simplify docker images and in general install and execution process.

When a gnoland node is started and no genesis files are specified, we will log a warning per genesis file indicating to the user that we are loading defaults.

Related with #2091

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

Proposal for embedding initial genesis data inside the gnoland binary
instead of need them on the filesystem in the gnoroot folder.

It will simplify docker images and in general install and execution
process.

Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro ajnavarro requested review from moul, gfanton, zivkovicmilos and a team as code owners May 16, 2024 13:43
@github-actions github-actions bot added the 📦 ⛰️ gno.land Issues or PRs gno.land package related label May 16, 2024
Copy link

codecov bot commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.08%. Comparing base (ccc6d5b) to head (8e7667b).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2129      +/-   ##
==========================================
+ Coverage   49.04%   49.08%   +0.04%     
==========================================
  Files         576      576              
  Lines       77556    77570      +14     
==========================================
+ Hits        38035    38079      +44     
+ Misses      36439    36404      -35     
- Partials     3082     3087       +5     
Flag Coverage Δ
gno.land 62.53% <100.00%> (+0.84%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
// Initialize the zap logger
zapLogger := log.GetZapLoggerFn(logFormat)(io.Out(), logLevel)

// Wrap the zap logger
logger := log.ZapLoggerToSlog(zapLogger)

slog.SetDefault(logger)
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about this. I don't care if we use the slog default logger, but we should use it consistently throughout or not use it at all. I don't want to mix slog.Default and non-default logger.

@ajnavarro
Copy link
Contributor Author

Not clear if these files are something we should embed into the bin, or completely avoid having them on the first place.

@ajnavarro ajnavarro closed this May 29, 2024
@thehowl
Copy link
Member

thehowl commented May 30, 2024

This could be fine as an interim default, and I think we can merge this PR if you want @ajnavarro, but eventually these files should be removed with the chain initialization efforts (in test4).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 ⛰️ gno.land Issues or PRs gno.land package related
Projects
Status: Done
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants