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.

Sign up for GitHub

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

Jump to bottom

Enable the Beta RPC API (v2) for all unit tests: #4573

Merged
merged 1 commit into from Jun 21, 2023
Merged

Enable the Beta RPC API (v2) for all unit tests: #4573

merged 1 commit into from Jun 21, 2023

Conversation

ximinez
Copy link
Collaborator

@ximinez ximinez commented Jun 13, 2023

High Level Overview of Change

I noticed recently a handful of PRs (#4552, #4568, and #4571) that are changing the apiMaximumSupportedVersion value from 1 to 2, which would indicate that version two is finalized and out of beta, apparently only to enable API version testing in unit tests.

IMHO, the proper way to enable versioning in unit tests is to enable the BETA_RPC_API flag in the config. For simplicity, this PR sets that flag in the default unit test config, so now the beta (v2) is available to all unit tests. The default will still be 1, but any new unit tests should be able to specify an API version and work as expected without any hassle.

Type of Change

  • [X ] Tests (You added tests for code that already exists, or your new feature included in this PR)

Test Plan

As this code only effects unit tests, the only testing necessary should be to verify that unit tests pass.

@ximinez
Enable the Beta RPC API (v2) for all unit tests:
ef0aeca 
* Doesn't change any defaults.
* The only existing tests changed were one that set the same flag, which
  was now redundant, and a couple that tested versioning explicitly.
@ximinez
Copy link
Collaborator Author

ximinez commented Jun 13, 2023

I nominate the authors of the aforementioned PRs as reviewers for this one: @PeterChen13579 and @arihantkothari.

This was referenced Jun 13, 2023
Merged
Closed
Merged
@intelliot intelliot added this to the 1.12 milestone Jun 14, 2023
@intelliot intelliot merged commit 71d7d67 into XRPLF:develop Jun 21, 2023
15 checks passed
@ximinez ximinez deleted the testBetaApi branch June 21, 2023 19:00
arihantkothari added a commit to arihantkothari/rippled that referenced this pull request Jun 22, 2023
@arihantkothari
updating based on XRPLF#4573 PR
50cbc1c
@arihantkothari arihantkothari mentioned this pull request Jun 22, 2023
Closed
1 task
arihantkothari added a commit to arihantkothari/rippled that referenced this pull request Jul 4, 2023
@arihantkothari
APIv2(ledger_entry): return "invalidParams" when fields missing (XRPL…
90b04cb 
…F#4552)

Improve error handling for ledger_entry by returning an "invalidParams"
error when one or more request fields are specified incorrectly, or one
or more required fields are missing.

For example, if none of of the following fields is provided, then the
API should return an invalidParams error:
* index, account_root, directory, offer, ripple_state, check, escrow,
  payment_channel, deposit_preauth, ticket

Prior to this commit, the API returned an "unknownOption" error instead.
Since the error was actually due to invalid parameters, rather than
unknown options, this error was misleading.

Since this is an API breaking change, the "invalidParams" error is only
returned for requests using api_version: 2 and above. To maintain
backward compatibility, the "unknownOption" error is still returned for
api_version: 1.

Related: XRPLF#4573

Fix XRPLF#4303
intelliot pushed a commit that referenced this pull request Jul 6, 2023
@arihantkothari
APIv2(ledger_entry): return "invalidParams" when fields missing (#4552)
a45a95e 
Improve error handling for ledger_entry by returning an "invalidParams"
error when one or more request fields are specified incorrectly, or one
or more required fields are missing.

For example, if none of of the following fields is provided, then the
API should return an invalidParams error:
* index, account_root, directory, offer, ripple_state, check, escrow,
  payment_channel, deposit_preauth, ticket

Prior to this commit, the API returned an "unknownOption" error instead.
Since the error was actually due to invalid parameters, rather than
unknown options, this error was misleading.

Since this is an API breaking change, the "invalidParams" error is only
returned for requests using api_version: 2 and above. To maintain
backward compatibility, the "unknownOption" error is still returned for
api_version: 1.

Related: #4573

Fix #4303
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Jul 10, 2023
@ximinez @ckeshava
Enable the Beta RPC API (v2) for all unit tests: (XRPLF#4573)
bd85cfa 
* Enable api_version 2, which is currently in beta. It is expected to be
  marked stable by the next stable release.
* This does not change any defaults.
* The only existing tests changed were one that set the same flag, which
  was now redundant, and a couple that tested versioning explicitly.
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Jul 10, 2023
@arihantkothari @ckeshava
APIv2(ledger_entry): return "invalidParams" when fields missing (XRPL…
340b0b5 
…F#4552)

Improve error handling for ledger_entry by returning an "invalidParams"
error when one or more request fields are specified incorrectly, or one
or more required fields are missing.

For example, if none of of the following fields is provided, then the
API should return an invalidParams error:
* index, account_root, directory, offer, ripple_state, check, escrow,
  payment_channel, deposit_preauth, ticket

Prior to this commit, the API returned an "unknownOption" error instead.
Since the error was actually due to invalid parameters, rather than
unknown options, this error was misleading.

Since this is an API breaking change, the "invalidParams" error is only
returned for requests using api_version: 2 and above. To maintain
backward compatibility, the "unknownOption" error is still returned for
api_version: 1.

Related: XRPLF#4573

Fix XRPLF#4303
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 22, 2023
@ximinez @ckeshava
Enable the Beta RPC API (v2) for all unit tests: (XRPLF#4573)
5e27424 
* Enable api_version 2, which is currently in beta. It is expected to be
  marked stable by the next stable release.
* This does not change any defaults.
* The only existing tests changed were one that set the same flag, which
  was now redundant, and a couple that tested versioning explicitly.
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 22, 2023
@arihantkothari @ckeshava
APIv2(ledger_entry): return "invalidParams" when fields missing (XRPL…
0fa1b97 
…F#4552)

Improve error handling for ledger_entry by returning an "invalidParams"
error when one or more request fields are specified incorrectly, or one
or more required fields are missing.

For example, if none of of the following fields is provided, then the
API should return an invalidParams error:
* index, account_root, directory, offer, ripple_state, check, escrow,
  payment_channel, deposit_preauth, ticket

Prior to this commit, the API returned an "unknownOption" error instead.
Since the error was actually due to invalid parameters, rather than
unknown options, this error was misleading.

Since this is an API breaking change, the "invalidParams" error is only
returned for requests using api_version: 2 and above. To maintain
backward compatibility, the "unknownOption" error is still returned for
api_version: 1.

Related: XRPLF#4573

Fix XRPLF#4303
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 25, 2023
@ximinez @ckeshava
Enable the Beta RPC API (v2) for all unit tests: (XRPLF#4573)
31ec0d6 
* Enable api_version 2, which is currently in beta. It is expected to be
  marked stable by the next stable release.
* This does not change any defaults.
* The only existing tests changed were one that set the same flag, which
  was now redundant, and a couple that tested versioning explicitly.
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 25, 2023
@arihantkothari @ckeshava
APIv2(ledger_entry): return "invalidParams" when fields missing (XRPL…
58ce445 
…F#4552)

Improve error handling for ledger_entry by returning an "invalidParams"
error when one or more request fields are specified incorrectly, or one
or more required fields are missing.

For example, if none of of the following fields is provided, then the
API should return an invalidParams error:
* index, account_root, directory, offer, ripple_state, check, escrow,
  payment_channel, deposit_preauth, ticket

Prior to this commit, the API returned an "unknownOption" error instead.
Since the error was actually due to invalid parameters, rather than
unknown options, this error was misleading.

Since this is an API breaking change, the "invalidParams" error is only
returned for requests using api_version: 2 and above. To maintain
backward compatibility, the "unknownOption" error is still returned for
api_version: 1.

Related: XRPLF#4573

Fix XRPLF#4303
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@intelliot intelliot intelliot approved these changes

@PeterChen13579 PeterChen13579 PeterChen13579 approved these changes

@arihantkothari arihantkothari arihantkothari approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Core Ledger
Archived in project
Milestone
1.12
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ximinez @intelliot @PeterChen13579 @arihantkothari