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

[Clock] Add timezone config YANG model #14651

Merged
merged 3 commits into from
Jun 7, 2023
Merged

[Clock] Add timezone config YANG model #14651

merged 3 commits into from
Jun 7, 2023

Conversation

fastiuk
Copy link
Contributor

@fastiuk fastiuk commented Apr 13, 2023 •
edited
Loading

hld #1219
closes #1171
depends-on #2793, #57

Why I did it

  • To be able to configure timezone
  • To be able to show timezones

How I did it

  • Add YANG model
  • Add default value to init_cfg.json

How to verify it

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211

Tested branch (Please provide the tested image version)

Description for the changelog

  • Add timezone config YANG model

Link to config_db schema for YANG module changes

Device metadata configuration

A picture of a cute animal (it is my cat Finn)

PXL_20230413_140941610 PORTRAIT

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 13, 2023 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: fastiuk / name: Yevhen Fastiuk (dce29e6372b2bbad0726c39e7f9ab576948b17f9, 59c6f2e8e7212688667ddc64cceba58e2cec4842, f512d4aa5ce550398c4f48f25cc46db7d898faf6)

@dgsudharsan
Copy link
Collaborator

dgsudharsan commented Apr 13, 2023 •
edited
Loading

@fastiuk Can you please update correct PR in the description? I believe it should be sonic-net/SONiC#1171

dgsudharsan
dgsudharsan previously approved these changes Apr 13, 2023
View reviewed changes
@dgsudharsan dgsudharsan added the YANG YANG model related changes label Apr 13, 2023
@fastiuk
Copy link
Contributor Author

fastiuk commented Apr 13, 2023

@fastiuk Can you please update correct PR in the description? I believe it should be sonic-net/SONiC#1171

I did already. I didn't notice the issue at the beginning

@fastiuk fastiuk self-assigned this Apr 13, 2023
dgsudharsan
dgsudharsan previously approved these changes Apr 14, 2023
View reviewed changes
@Meir-renford Meir-renford mentioned this pull request Apr 18, 2023
Merged
@fastiuk fastiuk changed the title [Clock] Implement clock CLI [Clock] Add timezone config YANG model May 2, 2023
@fastiuk
Copy link
Contributor Author

fastiuk commented May 2, 2023

Rebased to resolve conflicts

@fastiuk fastiuk closed this May 7, 2023
@fastiuk fastiuk reopened this May 7, 2023
@fastiuk
Copy link
Contributor Author

fastiuk commented May 7, 2023

Ping here

@fastiuk fastiuk closed this May 10, 2023
@fastiuk fastiuk reopened this May 10, 2023
yxieca
yxieca previously approved these changes May 15, 2023
View reviewed changes
@fastiuk
Copy link
Contributor Author

fastiuk commented May 22, 2023

Rebased due to conflicts
@yxieca could you please press approve button again?

@fastiuk fastiuk requested a review from yxieca May 22, 2023 11:36
@fastiuk fastiuk closed this May 22, 2023
@fastiuk fastiuk reopened this May 22, 2023
@fastiuk fastiuk closed this May 23, 2023
@fastiuk fastiuk reopened this May 23, 2023
dgsudharsan
dgsudharsan previously approved these changes May 24, 2023
View reviewed changes
@fastiuk
Add the ability to configure timezone
2586ca6 
Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>
@fastiuk
Add YANG model for timezone
b797ac1 
Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>
@fastiuk
Add timezone reference
60695c2 
Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>
@fastiuk
Copy link
Contributor Author

fastiuk commented May 26, 2023

@yxieca I replaced Etc/UTC with UTC. Please reapprove

@fastiuk fastiuk closed this May 26, 2023
@fastiuk fastiuk reopened this May 26, 2023
@fastiuk fastiuk closed this May 30, 2023
@fastiuk fastiuk reopened this May 30, 2023
@qiluo-msft qiluo-msft requested review from ganglyu and wen587 June 5, 2023 16:51
ganglyu
ganglyu reviewed Jun 6, 2023
View reviewed changes
src/sonic-yang-models/yang-models/sonic-device_metadata.yang
leaf timezone {
type stypes:timezone-name-type {
length 1..255;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not move length to sonic-types.yang?

Copy link
Contributor

@wen587 wen587 Jun 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

agreed. Others look good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

timezone-name-type was copied from the Openconfig. I prefer to leave it unchanged there. So that is why I add the length definition here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really need length?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we shouldn't allow empty timezones, as well as enormous long strings, like more than 255 characters long.

@yxieca yxieca merged commit 8a6d452 into sonic-net:master Jun 7, 2023
sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
@fastiuk @sonic-otn
[Clock] Add timezone config YANG model (sonic-net#14651)
68ca423 
* Add the ability to configure timezone

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

* Add YANG model for timezone

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

* Add timezone reference

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

---------

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgsudharsan dgsudharsan dgsudharsan approved these changes

@ganglyu ganglyu ganglyu approved these changes

@qiluo-msft qiluo-msft Awaiting requested review from qiluo-msft qiluo-msft is a code owner

@lguohan lguohan Awaiting requested review from lguohan lguohan is a code owner

@yxieca yxieca Awaiting requested review from yxieca

@wen587 wen587 Awaiting requested review from wen587

Assignees

@fastiuk fastiuk

Labels
YANG YANG model related changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Standalone local clock setting
5 participants
@fastiuk @dgsudharsan @wen587 @yxieca @ganglyu