Add setup for the tool usage #59

nivcertora · 2024-12-31T14:39:16Z

https://certora.atlassian.net/browse/CERT-7888

Title: Refactor Configuration Management and Introduce Setup Command

Description:

This Pull Request introduces key enhancements to the Quorum project, focusing on improving configuration management, streamlining the setup process, and enhancing the overall codebase structure. The changes aim to provide a more efficient onboarding experience, ensure better code organization, and maintain high performance standards.

Key Changes:

Configuration Management:
- Renamed Configuration Module:
  - Renamed Quorum/config.py to Quorum/utils/config.py to better reflect its utility purpose.
- Adopted JSON5:
  - Replaced the standard json module with json5 across multiple files to allow for more flexible JSON parsing (e.g., comments, trailing commas).
  - Updated exception handling to accommodate JSONDecodeError and ValueError.
- Removed Automatic Config Copying:
  - Eliminated the automatic copying of ground_truth.json and execution.json within the configuration script.
  - Introduced explicit error handling to raise FileNotFoundError if essential configuration files are missing.
Setup Command Introduction:
- New Entry Point SetupQuorum:
  - Added SetupQuorum as a new CLI entry point in setup.py, enabling users to initialize their Quorum environment effortlessly.
- Added setup_quorum.py:
  - Created a dedicated setup script that automates the copying of essential template files (ground_truth.json, execution.json, .env.example, Readme.md) to the specified working directory.
  - Ensures that existing files are not overwritten, providing warnings to prevent accidental data loss.
CLI Enhancements:
- Moved Scripts to entry_points:
  - Relocated existing scripts (check_proposal.py, create_report.py, ipfs_validator.py) to the Quorum/entry_points/ directory for better organization.
- Updated CLI Commands:
  - Updated the console_scripts in setup.py to reflect the new locations of the entry point scripts.
CI Pipeline Adjustments:
- Updated CI.yml:
  - Changed QUORUM_PATH from . to Quorum/tests to align with the new project structure.
  - Modified the path to regression.json in the regression tests to Quorum/tests/regression.json.
Dependency Updates:
- Added json5 to requirements.txt:
  - Included json5 as a dependency to support the new JSON parsing requirements.
Documentation Updates:
- Enhanced README.md:
  - Updated the README to include detailed instructions for the new SetupQuorum command.
  - Removed redundant sections that are now covered by the setup command.
  - Added a Quick Setup section to guide users through the initial configuration process using the new setup script.
Template Management:
- Added templates Directory:
  - Introduced a Quorum/templates/ directory containing template configuration files (ground_truth.json, execution.json, .env.example, Readme.md) to standardize setup procedures.

…dd-Setup

Quorum/utils/config.py

README.md

Quorum/templates/Readme.md

Quorum/entry_points/setup_quorum.py

Co-authored-by: yoav-el-certora <[email protected]>

…nto niv/CERT-7888-Add-Setup

Quorum/templates/Readme.md

liav-certora

Minor naming changes to avoid future confusions.

I think using json5 is unnecessary since you are using it because of the templates, but:

You shouldn't open these template files, just copy them for the user (correct me if I'm wrong).
The user has to configure these templates either and they will most likely remove the comments because the files are formatted as json and their IDE will tell them there are errors.
This introduces another third party dependency which we don't need.

Repo organization looks great. Setup tool implementation is clean.

liav-certora · 2025-01-01T22:07:58Z

Quorum/apis/price_feeds/price_feed_utils.py

@@ -3,7 +3,7 @@
 from typing import Optional
 from pydantic import BaseModel, Field
 from pathlib import Path
-import json
+import json5 as json


Please change that to simply import json5. I'm afraid that down the line it will get confusing because these 2 packages do act a little bit differently in certain things (as you saw).

The difference between them is agnostic, if it will cause any issue in the future we can change it. for now, i prefer it to be called json instead of a variable with number name just for apperance.

liav-certora · 2025-01-01T22:08:05Z

Quorum/auto_report/aave_tags.py

@@ -1,6 +1,6 @@
 import requests
 from dataclasses import dataclass
-import json
+import json5 as json


liav-certora · 2025-01-01T22:08:11Z

Quorum/checks/check.py

@@ -1,9 +1,9 @@
 from abc import ABC
 from datetime import datetime
-import json
+import json5 as json


liav-certora · 2025-01-01T22:08:57Z

Quorum/entry_points/check_proposal.py

@@ -1,5 +1,6 @@
 import argparse
-import json
+from json import JSONDecodeError
+import json5 as json


liav-certora · 2025-01-01T22:10:40Z

Quorum/entry_points/setup_quorum.py

+import shutil
+import argparse
+from pathlib import Path
+import Quorum.utils.pretty_printer as pp


Blank line above please :)

liav-certora · 2025-01-01T22:33:16Z

Quorum/entry_points/setup_quorum.py

+    for file_name in template_files:
+        src = templates_dir / file_name
+        dest = target_dir / '.env' if file_name == '.env.example' else target_dir / file_name
+
+        if dest.exists():
+            pp.pretty_print(f"File exists: {dest}. Skipping.", pp.Colors.WARNING)
+            continue
+
+        shutil.copy(src, dest)
+        pp.pretty_print(f"Copied {file_name} to {dest}", pp.Colors.SUCCESS)


I checked and if .env is in a subdirectory it won't be found automatically so you can rename .env.example to .env without worrying. With that you can just use shutil.copytree instead of the whole for loop but that means everything will be overridden, unlike how you skip existing files. Which I actually think can be better because if a user messes his directory or one of the files he can easily just run this tool again.
You have the option, whatever you prefer.

I prefer the current state, and also i would suggest to not upload a .env to the repo.

liav-certora · 2025-01-01T22:37:46Z

Quorum/tests/expected/test_auto_report/v3-132.md

I already told you that I think changing the expected file instead of changing the code is principally wrong. Yes even for the smallest of differences like ", especially when the fix involves adding 3 words.

I understand your point and yet if the output is improved i am not seeing any issue with modify the test for the better output.

liav-certora · 2025-01-01T22:37:56Z

Quorum/tests/test_price_feed_providers.py

@@ -5,7 +5,7 @@
 from Quorum.utils.chain_enum import Chain
 from Quorum.apis.price_feeds import PriceFeedData, ChainLinkAPI, ChronicleAPI, CoinGeckoAPI

-import json
+import json5 as json


liav-certora · 2025-01-01T22:38:03Z

Quorum/tests/test_source_code.py

@@ -4,7 +4,7 @@

 from Quorum.apis.block_explorers.source_code import SourceCode

-import json
+import json5 as json


liav-certora · 2025-01-01T22:38:35Z

Quorum/utils/config_loader.py

@@ -1,6 +1,6 @@
-import json
+import json5 as json


nivcertora · 2025-01-02T05:59:29Z

Minor naming changes to avoid future confusions.

I think using json5 is unnecessary since you are using it because of the templates, but:

You shouldn't open these template files, just copy them for the user (correct me if I'm wrong).

The user has to configure these templates either and they will most likely remove the comments because the files are formatted as json and their IDE will tell them there are errors.
This introduces another third party dependency which we don't need.

Repo organization looks great. Setup tool implementation is clean.

I honestly don't see any difference in terms of using json5 over json. as long as it dosent introduce any issues its preferred to be used just in case the user will leave comments in the templates instead of crashing the tool. and most users use sublime text and not a real ide for just general knowledge.

yoav-el-certora · 2025-01-02T06:53:19Z

Quorum/entry_points/setup_quorum.py

+    template_files = [f.name for f in templates_dir.iterdir() if f.is_file()]
+
+    for file_name in template_files:
+        src = templates_dir / file_name
+        dest = target_dir / '.env' if file_name == '.env.example' else target_dir / file_name
+
+        if dest.exists():
+            pp.pretty_print(f"File exists: {dest}. Skipping.", pp.Colors.WARNING)
+            continue
+
+        shutil.copy(src, dest)
+        pp.pretty_print(f"Copied {file_name} to {dest}", pp.Colors.SUCCESS)


Suggested change

template_files = [f.name for f in templates_dir.iterdir() if f.is_file()]

for file_name in template_files:

src = templates_dir / file_name

dest = target_dir / '.env' if file_name == '.env.example' else target_dir / file_name

if dest.exists():

pp.pretty_print(f"File exists: {dest}. Skipping.", pp.Colors.WARNING)

continue

shutil.copy(src, dest)

pp.pretty_print(f"Copied {file_name} to {dest}", pp.Colors.SUCCESS)

template_files = [f for f in templates_dir.iterdir() if f.is_file()]

for file in template_files:

dest = target_dir / '.env' if file.name == '.env.example' else target_dir / file.name

if dest.exists():

pp.pretty_print(f"File exists: {dest}. Skipping.", pp.Colors.WARNING)

continue

shutil.copy(file, dest)

pp.pretty_print(f"Copied {file.name} to {dest}", pp.Colors.SUCCESS)

Last one since functionality changed

Not sure its going to work for the .env check. so ill keep it for now

It will work as you iterate on absolute files

Quorum/entry_points/setup_quorum.py

Add setup for the tool usage

70f4738

nivcertora self-assigned this Dec 31, 2024

nivcertora requested a review from a team as a code owner December 31, 2024 14:39

nivcertora added 7 commits December 31, 2024 16:40

Merge branch 'main' of github.com:Certora/Quorum into niv/CERT-7888-A…

4bb3831

…dd-Setup

Fix tests

7909978

Try to fix regression

71fa3e3

Fix tests

f59e419

Fix tests

fd1ed0b

Add to README

0c26368

Autocomelet path

fd6660d

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

Quorum/utils/config.py Outdated Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

README.md Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

README.md Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

README.md Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

Quorum/templates/Readme.md Outdated Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

Quorum/templates/Readme.md Outdated Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

Quorum/templates/Readme.md Outdated Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

Quorum/templates/Readme.md Outdated Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

Quorum/entry_points/setup_quorum.py Outdated Show resolved Hide resolved

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

Quorum/entry_points/setup_quorum.py Outdated Show resolved Hide resolved

nivcertora and others added 7 commits January 1, 2025 15:08

Address comments

2748883

Update README.md

66d3482

Co-authored-by: yoav-el-certora <[email protected]>

Update README.md

4030cf1

Co-authored-by: yoav-el-certora <[email protected]>

Address comments

3d066cb

Update Quorum/templates/Readme.md

c2faa85

Co-authored-by: yoav-el-certora <[email protected]>

Update Quorum/templates/Readme.md

18eaa88

Co-authored-by: yoav-el-certora <[email protected]>

Update Quorum/templates/Readme.md

a3679c8

Co-authored-by: yoav-el-certora <[email protected]>

nivcertora and others added 2 commits January 1, 2025 15:16

Update Quorum/entry_points/setup_quorum.py

48850d8

Co-authored-by: yoav-el-certora <[email protected]>

Address comments

f4bb555

nivcertora requested a review from yoav-el-certora January 1, 2025 13:18

nivcertora added 2 commits January 1, 2025 15:19

Merge branch 'niv/CERT-7888-Add-Setup' of github.com:Certora/Quorum i…

4b21f76

…nto niv/CERT-7888-Add-Setup

Address comments

c65e33b

yoav-el-certora reviewed Jan 1, 2025

View reviewed changes

Quorum/templates/Readme.md Outdated Show resolved Hide resolved

Delete Quorum/templates/Readme.md

7cf025e

nivcertora requested review from yoav-el-certora and liav-certora January 1, 2025 15:25

liav-certora requested changes Jan 1, 2025

View reviewed changes

nivcertora added 2 commits January 2, 2025 08:03

Blank line done

88947a8

Collect template file automatic

97fc36d

nivcertora requested a review from liav-certora January 2, 2025 06:13

yoav-el-certora reviewed Jan 2, 2025

View reviewed changes

Quorum/entry_points/setup_quorum.py Show resolved Hide resolved

yoav-el-certora approved these changes Jan 2, 2025

View reviewed changes

nivcertora merged commit 5ef29c5 into main Jan 2, 2025
12 checks passed

nivcertora deleted the niv/CERT-7888-Add-Setup branch January 2, 2025 08:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add setup for the tool usage #59

Add setup for the tool usage #59

nivcertora commented Dec 31, 2024 •

edited

Loading

liav-certora left a comment

liav-certora Jan 1, 2025

nivcertora Jan 2, 2025

liav-certora Jan 1, 2025

nivcertora Jan 2, 2025

liav-certora Jan 1, 2025

nivcertora Jan 2, 2025

liav-certora Jan 1, 2025

nivcertora Jan 2, 2025

liav-certora Jan 1, 2025

nivcertora Jan 2, 2025

liav-certora Jan 1, 2025

nivcertora Jan 2, 2025

liav-certora Jan 1, 2025

nivcertora Jan 2, 2025

liav-certora Jan 1, 2025

nivcertora Jan 2, 2025

liav-certora Jan 1, 2025

nivcertora Jan 2, 2025

liav-certora Jan 1, 2025

nivcertora Jan 2, 2025

nivcertora commented Jan 2, 2025 •

edited

Loading

yoav-el-certora Jan 2, 2025

nivcertora Jan 2, 2025

yoav-el-certora Jan 2, 2025

Add setup for the tool usage #59

Add setup for the tool usage #59

Conversation

nivcertora commented Dec 31, 2024 • edited Loading

liav-certora left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nivcertora commented Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nivcertora commented Dec 31, 2024 •

edited

Loading

nivcertora commented Jan 2, 2025 •

edited

Loading