-
Notifications
You must be signed in to change notification settings - Fork 106
Update rippled.cfg file #824
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
base: main
Are you sure you want to change the base?
Changes from all commits
bff1aa9
4469f7b
0b0f8a0
82693e6
a53ed6c
cc6b797
4236ca6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,9 @@ jobs: | |
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v4 | ||
with: | ||
repository: ${{ github.event.pull_request.head.repo.full_name }} | ||
ref: ${{ github.event.pull_request.head.ref }} | ||
|
||
- name: Load cached .local | ||
id: cache-poetry | ||
|
@@ -30,10 +33,6 @@ jobs: | |
path: /home/runner/.local | ||
key: dotlocal-${{ env.POETRY_VERSION }} | ||
|
||
- name: Run docker in background | ||
run: | | ||
docker run --detach --rm -p 5005:5005 -p 6006:6006 --volume "${{ github.workspace }}/.ci-config/":"/etc/opt/ripple/" --name rippled-service --health-cmd="rippled server_info || exit 1" --health-interval=5s --health-retries=10 --health-timeout=2s --env GITHUB_ACTIONS=true --env CI=true --entrypoint bash ${{ env.RIPPLED_DOCKER_IMAGE }} -c "rippled -a" | ||
|
||
- name: Install poetry | ||
if: steps.cache-poetry.outputs.cache-hit != 'true' | ||
run: | | ||
|
@@ -53,6 +52,26 @@ jobs: | |
- name: Install poetry dependencies | ||
run: poetry install | ||
|
||
- name: Fetch latest rippled amendments | ||
if: ${{ github.event_name == 'pull_request' }} | ||
run: | | ||
poetry run python tools/fetch_rippled_amendments.py | ||
|
||
- name: Add and commit rippled.cfg | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why run this as a part of integration tests rather than have a separate PR open on a cron job to update it? Feels potentially confusing to anyone submitting a PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Integration tests make use of a docker container. The
Are you suggesting Github Actions cron jobs (or) running a cron-job from my personal machine to periodically fetch rippled PRs ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A GitHub Action cron job. |
||
if: ${{ github.event_name == 'pull_request' }} | ||
uses: EndBug/add-and-commit@v9 | ||
with: | ||
message: 'Update rippled.cfg with latest amendments' | ||
author_name: GitHub Actions | ||
author_email: [email protected] | ||
committer_name: GitHub Actions | ||
committer_email: [email protected] | ||
add: .ci-config/rippled.cfg | ||
|
||
- name: Run docker in background | ||
run: | | ||
docker run --detach --rm -p 5005:5005 -p 6006:6006 --volume "${{ github.workspace }}/.ci-config/":"/etc/opt/ripple/" --name rippled-service --health-cmd="rippled server_info || exit 1" --health-interval=5s --health-retries=10 --health-timeout=2s --env GITHUB_ACTIONS=true --env CI=true --entrypoint bash ${{ env.RIPPLED_DOCKER_IMAGE }} -c "rippled -a" | ||
|
||
- name: Integration test | ||
run: | | ||
poetry run poe test_integration | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
""" | ||
This script fetches the latest amendments from (the `develop` branch of) rippled and | ||
adds them to the `rippled.cfg` file. | ||
""" | ||
|
||
import configparser | ||
import os | ||
import re | ||
import sys | ||
|
||
import requests | ||
|
||
CONFIG_FILE = os.path.join(os.getcwd(), ".ci-config", "rippled.cfg") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be much more efficient to use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are cases where new rippled amendments have been merged into the cpp code, but haven't been deployed into a devnet yet. I suspect this happens frequently for new amendments. Parsing the I'm not strongly opposed to using the |
||
FEATURES_SECTION = "features" | ||
RIPPLED_FEATURES_FILE = "https://raw.githubusercontent.com/XRPLF/rippled/develop/include/xrpl/protocol/detail/features.macro" | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
config = configparser.ConfigParser( | ||
allow_no_value=True, | ||
) | ||
config.optionxform = str # type: ignore | ||
config.read(CONFIG_FILE) | ||
|
||
|
||
def fetch_rippled_amendments(): | ||
response = requests.get(RIPPLED_FEATURES_FILE, timeout=30) | ||
if response.status_code == 200: | ||
features_contents = response.text | ||
feature_hits = re.findall( | ||
r"^ *XRPL_FEATURE *\(([a-zA-Z0-9_]+), * Supported::yes, VoteBehavior::Default(?:Yes|No)", | ||
features_contents, | ||
re.MULTILINE, | ||
) | ||
|
||
fix_hits = re.findall( | ||
r"^ *XRPL_FIX *\(([a-zA-Z0-9_]+), * Supported::yes, VoteBehavior::Default(?:Yes|No)", | ||
features_contents, | ||
re.MULTILINE, | ||
) | ||
|
||
print(fix_hits) | ||
|
||
all_supported_amendments = feature_hits + ["fix" + f for f in fix_hits if f] | ||
return all_supported_amendments | ||
else: | ||
print(f"Failed to fetch file. Status code: {response.status_code}") | ||
|
||
|
||
if __name__ == "__main__": | ||
if FEATURES_SECTION in config: | ||
amendments_to_add = [] | ||
existing_amendments = [v for v in config[FEATURES_SECTION] if v] | ||
new_rippled_amendments = fetch_rippled_amendments() | ||
if new_rippled_amendments is None: | ||
print("ERROR: Failed to fetch rippled amendments.") | ||
sys.exit(1) | ||
|
||
for v in new_rippled_amendments: | ||
if v not in existing_amendments: | ||
amendments_to_add.append(v) | ||
|
||
if len(amendments_to_add) > 0: | ||
print( | ||
"INFO: The following amendments need to be inserted into the config file: " | ||
+ ", ".join(amendments_to_add) | ||
) | ||
|
||
for v in amendments_to_add: | ||
config.set(FEATURES_SECTION, v, value=None) | ||
|
||
with open(CONFIG_FILE, "w", encoding="utf-8") as rippled_cfg_file: | ||
config.write(rippled_cfg_file) | ||
else: | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
print(f"INFO: No new amendments to add into the {CONFIG_FILE} file.") | ||
else: | ||
print(f"ERROR: No features section found in the {CONFIG_FILE} file.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are all the comments removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've explained in point-2 under "Context of Change": #824 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comments are for us, not for rippled - they're supposed to be ignored by rippled