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

pallet-timestamp: UnixTime::now implementation logs error only if called at genesis #5055

Merged
merged 4 commits into from
Jul 30, 2024

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented Jul 17, 2024

This PR reverts the removal of an if statement merged recently, which affected test output verbosity of several pallets (e.g. staking, EPM, and potentially others).

More generally, the UnixTime::now implementation of the timestamp pallet should log an error only when called at the genesis block.

@gpestana gpestana self-assigned this Jul 17, 2024
@gpestana gpestana requested a review from a team as a code owner July 17, 2024 18:17
@gpestana gpestana added the R0-silent Changes should not be mentioned in any release notes label Jul 17, 2024
@bkchr
Copy link
Member

bkchr commented Jul 17, 2024

This PR reduces the unnecessary verbosity in non-std environments when calling UnixTime::now() implemented by the timestamp pallet.

About which environments are you talking here about?

@gpestana
Copy link
Contributor Author

gpestana commented Jul 17, 2024

About which environments are you talking here about?

feature = test, for example.

The error is polluting a bunch of test cases.

@bkchr
Copy link
Member

bkchr commented Jul 18, 2024

feature = test, for example.

Where is this run? In CI?

@gpestana
Copy link
Contributor Author

feature = test, for example.

Where is this run? In CI?

Yep, and locally too. For example, if you run the test of the pallet-staking in master locally, you'll see the error message logged from the timestamp pallet due to removing the sp_std::if_std! check. Now, the tests won't fail but the error log is output and annoying.

@gpestana gpestana changed the title pallet-timestamp: Reduces error verbosity in non-std environments pallet-timestamp: UnixTime::now logs error only if called at genesis Jul 30, 2024
@gpestana gpestana changed the title pallet-timestamp: UnixTime::now logs error only if called at genesis pallet-timestamp: UnixTime::now implementation logs error only if called at genesis Jul 30, 2024
@paritytech-review-bot paritytech-review-bot bot requested a review from a team July 30, 2024 14:14
@gpestana gpestana requested a review from bkchr July 30, 2024 14:16
@bkchr bkchr enabled auto-merge July 30, 2024 14:17
@ggwpez
Copy link
Member

ggwpez commented Jul 30, 2024

FYI R0-silent means that this change will not trigger a release of this crate.

@bkchr bkchr added T2-pallets This PR/Issue is related to a particular pallet. and removed R0-silent Changes should not be mentioned in any release notes labels Jul 30, 2024
@bkchr
Copy link
Member

bkchr commented Jul 30, 2024

Then @gpestana please add some prdoc.

@bkchr bkchr added this pull request to the merge queue Jul 30, 2024
Merged via the queue into master with commit 03c45b9 Jul 30, 2024
159 of 162 checks passed
@bkchr bkchr deleted the gpestana/timestamp_err_verbosity branch July 30, 2024 15:31
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…alled at genesis (paritytech#5055)

This PR reverts the removal of an [`if`
statement](paritytech@7ecf3f7#diff-8bf31ba8d9ebd6377983fd7ecc7f4e41cb1478a600db1a15a578d1ae0e8ed435L370)
merged recently, which affected test output verbosity of several pallets
(e.g. staking, EPM, and potentially others).

More generally, the `UnixTime::now` implementation of the timestamp
pallet should log an error *only* when called at the genesis block.
ordian added a commit that referenced this pull request Aug 6, 2024
* master: (51 commits)
  Remove unused feature gated code from the minimal template (#5237)
  make polkadot-parachain startup errors pretty (#5214)
  Coretime auto-renew (#4424)
  network/strategy: Backoff and ban overloaded peers to avoid submitting the same request multiple times (#5029)
  Fix frame crate usage doc (#5222)
  beefy: Tolerate pruned state on runtime API call (#5197)
  rpc: Enable ChainSpec for polkadot-parachain (#5205)
  Add an adapter for configuring AssetExchanger (#5130)
  Replace env_logger with sp_tracing (#5065)
  Adjust sync templates flow to use new release branch (#5182)
  litep2p/discovery: Publish authority records with external addresses only (#5176)
  Run UI tests in CI for some other crates (#5167)
  Remove `pallet::getter` usage from the pallet-balances (#4967)
  pallet-timestamp: `UnixTime::now` implementation logs error only if called at genesis (#5055)
  [CI] Cache try-runtime check (#5179)
  [Backport] version bumps and the prdocs reordering from stable2407 (#5178)
  [subsystem-benchmark] Update availability-distribution-regression-bench baseline after recent subsystem changes (#5180)
  Remove pallet::getter usage from proxy (#4963)
  Remove pallet::getter macro usage from pallet-election-provider-multi-phase (#4487)
  [email protected] (#5177)
  ...
bkchr pushed a commit that referenced this pull request Aug 25, 2024
…alled at genesis (#5055)

This PR reverts the removal of an [`if`
statement](7ecf3f7#diff-8bf31ba8d9ebd6377983fd7ecc7f4e41cb1478a600db1a15a578d1ae0e8ed435L370)
merged recently, which affected test output verbosity of several pallets
(e.g. staking, EPM, and potentially others).

More generally, the `UnixTime::now` implementation of the timestamp
pallet should log an error *only* when called at the genesis block.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants