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

Remove the configuration of synchronous mode from init_cfg.json #5308

Merged
merged 6 commits into from
Sep 10, 2020

Conversation

shi-su
Copy link
Contributor

@shi-su shi-su commented Sep 3, 2020

- Why I did it
Remove the configuration of synchronous mode from init_cfg.json

- How I did it
Remove the configuration of synchronous mode from init_cfg.json by removing it from the template file.

- How to verify it
I verified it by testing locally with a virtual switch.

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@@ -573,7 +574,9 @@ def parse_meta(meta, hname):
region = value
elif name == "CloudType":
cloudtype = value
return syslog_servers, dhcp_servers, ntp_servers, tacacs_servers, mgmt_routes, erspan_dst, deployment_id, region, cloudtype
elif name == "SynchronousMode":
Copy link
Collaborator

@qiluo-msft qiluo-msft Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SynchronousMode [](start = 30, length = 15)

The input minigraph.xml has no new field for you to parse. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the field parser in minigraph .xml

@@ -230,7 +230,6 @@ $(info "INCLUDE_SFLOW" : "$(INCLUDE_SFLOW)")
$(info "INCLUDE_NAT" : "$(INCLUDE_NAT)")
$(info "INCLUDE_KUBERNETES" : "$(INCLUDE_KUBERNETES)")
$(info "TELEMETRY_WRITABLE" : "$(TELEMETRY_WRITABLE)")
$(info "ENABLE_SYNCHRONOUS_MODE" : "$(ENABLE_SYNCHRONOUS_MODE)")
Copy link
Collaborator

@qiluo-msft qiluo-msft Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ENABLE_SYNCHRONOUS_MODE [](start = 8, length = 23)

Just wondering if there is a need to build for async/sync for community members. #ByDesign

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel it is not a must as long as we provide the community with the CLI to change the configuration. The users can update the mode after booting up the image, and this scenario should be pretty rare.

@@ -538,6 +538,9 @@ sudo LANG=C cp $SCRIPTS_DIR/radv.sh $FILESYSTEM_ROOT/usr/local/bin/radv.sh
# Copy sonic-netns-exec script
sudo LANG=C cp $SCRIPTS_DIR/sonic-netns-exec $FILESYSTEM_ROOT/usr/bin/sonic-netns-exec

# Copy sonic-sync-mode script
sudo LANG=C cp $SCRIPTS_DIR/sonic-sync-mode $FILESYSTEM_ROOT/usr/bin/sonic-sync-mode
Copy link
Collaborator

@qiluo-msft qiluo-msft Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sonic-sync-mode [](start = 69, length = 15)

Does it make more sense to move this script to sonic-utilities repo? If yes, you don't need to manually install. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it to sonic-utilities repo.

formatter_class=argparse.RawTextHelpFormatter,
epilog=
'''
**sudo** needed for configuring synchronous mode
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sudo [](start = 0, length = 8)

Do you mean bold font? It's difficult to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed this and moved the CLI to sonic-utilities repo.

Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As comments

@shi-su shi-su changed the title Add CLI and minigraph parser for configuring synchronous mode Remove the configuration of synchronous mode from init_cfg.json Sep 3, 2020
@lguohan lguohan merged commit 339cfbf into sonic-net:master Sep 10, 2020
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
…c-net#5308)

Remove the configuration of synchronous mode from init_cfg.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants