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(cli): add argument to make sync-v1 unavailable #982

Merged
merged 1 commit into from
May 3, 2024

Conversation

jansegre
Copy link
Member

@jansegre jansegre commented Mar 21, 2024

Motivation

In preparation to phase-out sync-v1, we would benefit from having a method for starting the node without sync-v1 indexes. This PR does not go into the details of which index is used, it only makes it possible to start with a node where sync-v1 is "not available" as opposed to just "disabled"; "not available" means it cannot be enabled in runtime.

Acceptance Criteria

  • Refactor the builder/cli to make it clear to understand how each sync protocol is enabled or made available
  • Add --x-remove-sync-v1 CLI option to make sync-v1 unavailable from the start

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@jansegre jansegre self-assigned this Mar 21, 2024
@jansegre jansegre force-pushed the feat/arg-to-make-sync-v1-unavailable branch 2 times, most recently from de2ba65 to 9a20717 Compare March 22, 2024 16:21
Copy link

codecov bot commented Mar 22, 2024

Codecov Report

Attention: Patch coverage is 91.07143% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 84.93%. Comparing base (dc7071e) to head (3170851).

Files Patch % Lines
hathor/builder/cli_builder.py 83.33% 2 Missing and 2 partials ⚠️
hathor/builder/builder.py 96.77% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #982      +/-   ##
==========================================
+ Coverage   84.92%   84.93%   +0.01%     
==========================================
  Files         298      298              
  Lines       22952    22962      +10     
  Branches     3466     3470       +4     
==========================================
+ Hits        19492    19503      +11     
+ Misses       2774     2772       -2     
- Partials      686      687       +1     

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

@jansegre jansegre marked this pull request as ready for review April 1, 2024 19:43
@jansegre jansegre requested a review from msbrogli as a code owner April 1, 2024 19:43
@jansegre jansegre requested a review from glevco April 1, 2024 19:44
@jansegre jansegre force-pushed the feat/arg-to-make-sync-v1-unavailable branch from 9a20717 to 29eaa1a Compare April 1, 2024 19:45
hathor/builder/builder.py Outdated Show resolved Hide resolved
hathor/builder/cli_builder.py Outdated Show resolved Hide resolved
hathor/cli/run_node.py Outdated Show resolved Hide resolved
msbrogli
msbrogli previously approved these changes Apr 18, 2024
@jansegre jansegre force-pushed the feat/arg-to-make-sync-v1-unavailable branch from 29eaa1a to 78c5e79 Compare April 29, 2024 16:13
@jansegre jansegre requested review from glevco and msbrogli April 29, 2024 16:30
msbrogli
msbrogli previously approved these changes May 2, 2024
hathor/builder/cli_builder.py Outdated Show resolved Hide resolved
@jansegre jansegre force-pushed the feat/arg-to-make-sync-v1-unavailable branch from 78c5e79 to c641b44 Compare May 2, 2024 15:24
glevco
glevco previously approved these changes May 2, 2024
@jansegre jansegre force-pushed the feat/arg-to-make-sync-v1-unavailable branch from c641b44 to 57c9c39 Compare May 2, 2024 21:25
Currently it is possibly to disable sync-v1 but not make it unavailable.
Making it unavailble will be required in order to be able to disable the
indexes that it needs.

This commit does not bother on any of the effects that are possible when
sync-v1 is unavailable, only on making it possible to do so via a
command line argument.
@jansegre jansegre force-pushed the feat/arg-to-make-sync-v1-unavailable branch from 57c9c39 to 3170851 Compare May 2, 2024 21:26
@jansegre jansegre merged commit 007eb6e into master May 3, 2024
12 checks passed
@jansegre jansegre deleted the feat/arg-to-make-sync-v1-unavailable branch May 3, 2024 02:09
This was referenced May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants