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: swap legacy faucet implementation #1614

Merged
merged 16 commits into from
Mar 14, 2024

Conversation

zivkovicmilos
Copy link
Member

@zivkovicmilos zivkovicmilos commented Feb 1, 2024

Description

This PR removes the old faucet implementation to utilize the new gnolang/faucet.

Additional things done:

  • subnet throttler tidied, made thread safe
  • gnoweb faucet page fixed (invalid template values)

Breaking change:

  • Gnoweb now interacts with the faucet differently than it did before (POST request instead of a form submission).
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.

@zivkovicmilos zivkovicmilos self-assigned this Feb 1, 2024
@github-actions github-actions bot added the 📦 ⛰️ gno.land Issues or PRs gno.land package related label Feb 1, 2024
Copy link

codecov bot commented Mar 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.50%. Comparing base (3481a03) to head (0717991).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1614      +/-   ##
==========================================
+ Coverage   44.81%   47.50%   +2.69%     
==========================================
  Files         459      388      -71     
  Lines       67707    61375    -6332     
==========================================
- Hits        30342    29159    -1183     
+ Misses      34827    29774    -5053     
+ Partials     2538     2442      -96     

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

@zivkovicmilos zivkovicmilos marked this pull request as ready for review March 8, 2024 14:08
@zivkovicmilos zivkovicmilos requested review from moul, a team and gfanton as code owners March 8, 2024 14:08
@zivkovicmilos zivkovicmilos added the breaking change Functionality that contains breaking changes label Mar 8, 2024
Copy link
Member

@gfanton gfanton left a comment

Choose a reason for hiding this comment

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

Looks good! My only request would be to make subnetThrottler.Request thread-safe.

gno.land/cmd/gnofaucet/throttle.go Outdated Show resolved Hide resolved
gno.land/cmd/gnofaucet/throttle.go Outdated Show resolved Hide resolved
@zivkovicmilos
Copy link
Member Author

zivkovicmilos commented Mar 11, 2024

Well aware some DB tests are failing because of a mod change, looking into it now 🙏

Converting the PR to draft until I sort it

EDIT:

  • fixed in this commit: 9ea74c1
  • released faucet v0.1.3 that rolls back its go mod, and has updated features

@zivkovicmilos zivkovicmilos marked this pull request as draft March 11, 2024 10:53
@zivkovicmilos zivkovicmilos marked this pull request as ready for review March 13, 2024 23:13
Copy link
Contributor

@ajnavarro ajnavarro left a comment

Choose a reason for hiding this comment

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

A couple of minor comments were added.

gno.land/cmd/gnofaucet/middleware.go Outdated Show resolved Hide resolved
gno.land/cmd/gnofaucet/middleware.go Outdated Show resolved Hide resolved
gno.land/cmd/gnofaucet/throttle.go Outdated Show resolved Hide resolved
@zivkovicmilos zivkovicmilos merged commit 462397d into gnolang:master Mar 14, 2024
192 of 193 checks passed
@zivkovicmilos zivkovicmilos deleted the feat/new-faucet branch March 14, 2024 12:24
albttx pushed a commit to albttx/gno that referenced this pull request Mar 15, 2024
This PR removes the old faucet implementation to utilize the new
[gnolang/faucet](https://github.com/gnolang/faucet).

Additional things done:
- subnet throttler tidied, made thread safe
- gnoweb faucet page fixed (invalid template values)

Breaking change:
- Gnoweb now interacts with the faucet differently than it did before
(POST request instead of a form submission).

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] 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](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Functionality that contains breaking changes 📦 ⛰️ gno.land Issues or PRs gno.land package related
Projects
Status: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants