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

[slave.mk]: remove updategraph.service generation in slave.mk #3153

Merged
merged 4 commits into from
Jul 17, 2019
Merged

[slave.mk]: remove updategraph.service generation in slave.mk #3153

merged 4 commits into from
Jul 17, 2019

Conversation

theasianpianist
Copy link
Contributor

@theasianpianist theasianpianist commented Jul 13, 2019 •
edited
Loading

Signed-off-by: Lawrence Lee t-lale@microsoft.com

- What I did
Currently, updategraph.service is created from the template in files/build_templates at build time and placed in the main directory, but this file is unused and not cleaned up after the build finishes. This is redundant since sonic_debian_extension.sh generates and places a copy inside the built image.

- Description for the changelog

  • Stop updategraph.service from being redundantly generated inside slave.mk

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

[slave.mk]: remove updategraph after build
79befc3 
* removes updategraph.service after build is finished

Signed-off-by: Lawrence Lee <t-lale@microsoft.com>
@jleveque
Copy link
Contributor

@theasianpianist:Please add more to the description to explain why this change is necessary. Thanks!

pavel-shirshov
pavel-shirshov suggested changes Jul 13, 2019
View reviewed changes
@jleveque
Copy link
Contributor

Do we even need to generate updategraph.service here? Is the resulting file used anywhere? It is also generated in https://github.com/Azure/sonic-buildimage/blob/master/files/build_templates/sonic_debian_extension.j2#L198.

[slave.mk]: fix spacing
a188c1c 
* make tabs consistent between lines

Signed-off-by: Lawrence Lee <t-lale@microsoft.com>
@theasianpianist
Copy link
Contributor Author

Do we even need to generate updategraph.service here? Is the resulting file used anywhere? It is also generated in https://github.com/Azure/sonic-buildimage/blob/master/files/build_templates/sonic_debian_extension.j2#L198.

Actually now that you mention it I'm not sure how it got generated in the main directory in the first place, there's no rule for it or anything. I'm not sure how it got put there on my system, I'll have to take a further look

@jleveque
Copy link
Contributor

It's generated on line 647 above (https://github.com/Azure/sonic-buildimage/pull/3153/files/79befc306b83f8465f60f1187dc11d81e4f302da#diff-54252fe595f09e268f275810298a4beeL647). Not sure if this is necessary...

@theasianpianist
Copy link
Contributor Author

You're right, I don't think it is. Looks like sonic_debian_extension.sh places the generate file directly into the image.

@theasianpianist
Copy link
Contributor Author

theasianpianist commented Jul 13, 2019 •
edited
Loading

@jleveque should I just remove it from the Makefile altogether? I don't think it'll affect anything but I'll run a test build with it removed to check.

@jleveque
Copy link
Contributor

If you can prove that it is superfluous to generate it in slave.mk (it may be legacy code that didn't get cleaned up), then yes, you can remove it altogether. Just need to make sure that it gets generated, copied and installed properly in sonic_debian_extension.j2.

Lawrence Lee added 2 commits July 15, 2019 10:12
[slave.mk]:
cd65765 
* remove updategraph generation in slave.mk

Signed-off-by: Lawrence Lee <t-lale@microsoft.com>
[slave.mk]: merge commit
2dcbee1
@theasianpianist
Copy link
Contributor Author

Confirmed that the updategraph.service generated in slave.mk is unused and can be removed

@jleveque
Copy link
Contributor

@theasianpianist: Great! Can you also please update the PR title to match the new nature of the change in this PR?

jleveque
jleveque suggested changes Jul 15, 2019
View reviewed changes
Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

Please update PR title accordingly.

@theasianpianist theasianpianist changed the title [slave.mk]: remove updategraph after build [slave.mk]: remove updategraph.service generation in slave.mk Jul 15, 2019
@jleveque
Copy link
Contributor

Retest vs please

@theasianpianist
Copy link
Contributor Author

retest vs please

2 similar comments
@theasianpianist
Copy link
Contributor Author

retest vs please

@theasianpianist
Copy link
Contributor Author

retest vs please

theasianpianist
theasianpianist commented Jul 17, 2019
View reviewed changes
Copy link
Contributor Author

@theasianpianist theasianpianist left a comment

Choose a reason for hiding this comment

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

Tests completed

@jleveque jleveque merged commit 34b841e into sonic-net:master Jul 17, 2019
@theasianpianist theasianpianist deleted the pr-rm-updategraph branch May 13, 2021 23:11
mssonicbld added a commit that referenced this pull request Feb 8, 2024
@mssonicbld
[submodule] Update submodule sonic-utilities to the latest HEAD autom…
f49711f 
…atically (#18071)

#### Why I did it
src/sonic-utilities
```
* a3cf5c02 - (HEAD -> master, origin/master, origin/HEAD) Fix the sfputil treats page number as decimal instead of hexadecimal (#3153) (6 hours ago) [Kebo Liu]
* 167f9966 - [Mellanox] Add support of the nvidia-bluefield platform to generate-dump utility. (#3091) (20 hours ago) [Oleksandr Ivantsiv]
```
#### How I did it
#### How to verify it
#### Description for the changelog
mssonicbld added a commit that referenced this pull request Feb 15, 2024
@mssonicbld
[submodule] Update submodule sonic-utilities to the latest HEAD autom…
8cd9463 
…atically (#18081)

#### Why I did it
src/sonic-utilities
```
* b2125761 - (HEAD -> 202311, origin/202311) [chassis] fix show bgp summary when no neighbors are present on one ASIC (#3158) (2 days ago) [Arvindsrinivasan Lakshmi Narasimhan]
* 54595c1e - [202311]Fix the sfputil treats page number as decimal instead of hexadecimal (#3153) (#3160) (5 days ago) [Sudharsan Dhamal Gopalarathnam]
```
#### How I did it
#### How to verify it
#### Description for the changelog
sonic-otn pushed a commit to Weitang-Zheng/sonic-buildimage that referenced this pull request Mar 11, 2024
@mssonicbld @sonic-otn
[submodule] Update submodule sonic-utilities to the latest HEAD autom…
423c1b7 
…atically (sonic-net#18071)

#### Why I did it
src/sonic-utilities
```
* a3cf5c02 - (HEAD -> master, origin/master, origin/HEAD) Fix the sfputil treats page number as decimal instead of hexadecimal (sonic-net#3153) (6 hours ago) [Kebo Liu]
* 167f9966 - [Mellanox] Add support of the nvidia-bluefield platform to generate-dump utility. (sonic-net#3091) (20 hours ago) [Oleksandr Ivantsiv]
```
#### How I did it
#### How to verify it
#### Description for the changelog
saksarav-nokia pushed a commit to saksarav-nokia/sonic-buildimage that referenced this pull request Mar 12, 2024
@mssonicbld @saksarav-nokia
[submodule] Update submodule sonic-utilities to the latest HEAD autom…
9ef9c1d 
…atically (sonic-net#18071)

#### Why I did it
src/sonic-utilities
```
* a3cf5c02 - (HEAD -> master, origin/master, origin/HEAD) Fix the sfputil treats page number as decimal instead of hexadecimal (sonic-net#3153) (6 hours ago) [Kebo Liu]
* 167f9966 - [Mellanox] Add support of the nvidia-bluefield platform to generate-dump utility. (sonic-net#3091) (20 hours ago) [Oleksandr Ivantsiv]
```
#### How I did it
#### How to verify it
#### Description for the changelog
mssonicbld added a commit that referenced this pull request Jun 4, 2024
@mssonicbld
[submodule] Update submodule sonic-swss to the latest HEAD automatica…
f7d8615 
…lly (#19132)

#### Why I did it
src/sonic-swss
```
* 98012ed4 - (HEAD -> master, origin/master, origin/HEAD) [build-docker-sonic-vs] Allowing partial build for sairedis (#3177) (3 days ago) [Nikola Dancejic]
* 6568193c - Do not apply QoS mapping item on the switch until the object is created (#3163) (3 days ago) [Stephen Sun]
* 1876a306 - Revert "Added support for "UNDERLAY_SET_DSCP" and "UNDERLAY_SET_DSCPV6" table…" (#3175) (3 days ago) [Prince Sunny]
* 1f12a8db - [subnet decap] Add subnet decap rule based on overlay ECMP (#3153) (3 days ago) [Longxiang Lyu]
* 8e8fc66c - [ci] Migrate from sonicbld to sonicbld-1es to fix S360 alert. (#3171) (4 days ago) [Liu Shilong]
* 4ae8bfa3 - Read switch_id of fabric switch from config_db (#3102) (4 days ago) [Song Yuan]
* f7376636 - Add sai call to isolate/unisolate a fabric port (#3141) (4 days ago) [jfeng-arista]
* 7c7cece4 - [Chassis][voq] remote link down ECMP acceleration (#3150) (4 days ago) [Arvindsrinivasan Lakshmi Narasimhan]
* c7ecd7db - Add fabric port monitoring toggle check (#3132) (4 days ago) [jfeng-arista]
* 9ffbcd58 - Added support for "UNDERLAY_SET_DSCP" and "UNDERLAY_SET_DSCPV6" tables (#3145) (4 days ago) [siqbal1986]
* ef9dcdbe - Handle learning duplicate IPs on different VRFs (#3165) (5 days ago) [Lawrence Lee]
* cc5a02c4 - [ci] use sonictest instead of sonic-common-test #3170 (6 days ago) [Liu Shilong]
```
#### How I did it
#### How to verify it
#### Description for the changelog
arun1355492 pushed a commit to arun1355492/sonic-buildimage that referenced this pull request Jul 26, 2024
@mssonicbld @arun1355492
[submodule] Update submodule sonic-swss to the latest HEAD automatica…
4dcd4e9 
…lly (sonic-net#19132)

#### Why I did it
src/sonic-swss
```
* 98012ed4 - (HEAD -> master, origin/master, origin/HEAD) [build-docker-sonic-vs] Allowing partial build for sairedis (sonic-net#3177) (3 days ago) [Nikola Dancejic]
* 6568193c - Do not apply QoS mapping item on the switch until the object is created (sonic-net#3163) (3 days ago) [Stephen Sun]
* 1876a306 - Revert "Added support for "UNDERLAY_SET_DSCP" and "UNDERLAY_SET_DSCPV6" table…" (sonic-net#3175) (3 days ago) [Prince Sunny]
* 1f12a8db - [subnet decap] Add subnet decap rule based on overlay ECMP (sonic-net#3153) (3 days ago) [Longxiang Lyu]
* 8e8fc66c - [ci] Migrate from sonicbld to sonicbld-1es to fix S360 alert. (sonic-net#3171) (4 days ago) [Liu Shilong]
* 4ae8bfa3 - Read switch_id of fabric switch from config_db (sonic-net#3102) (4 days ago) [Song Yuan]
* f7376636 - Add sai call to isolate/unisolate a fabric port (sonic-net#3141) (4 days ago) [jfeng-arista]
* 7c7cece4 - [Chassis][voq] remote link down ECMP acceleration (sonic-net#3150) (4 days ago) [Arvindsrinivasan Lakshmi Narasimhan]
* c7ecd7db - Add fabric port monitoring toggle check (sonic-net#3132) (4 days ago) [jfeng-arista]
* 9ffbcd58 - Added support for "UNDERLAY_SET_DSCP" and "UNDERLAY_SET_DSCPV6" tables (sonic-net#3145) (4 days ago) [siqbal1986]
* ef9dcdbe - Handle learning duplicate IPs on different VRFs (sonic-net#3165) (5 days ago) [Lawrence Lee]
* cc5a02c4 - [ci] use sonictest instead of sonic-common-test sonic-net#3170 (6 days ago) [Liu Shilong]
```
#### How I did it
#### How to verify it
#### Description for the changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulnice paulnice paulnice approved these changes

@jleveque jleveque jleveque approved these changes

@pavel-shirshov pavel-shirshov pavel-shirshov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@theasianpianist @jleveque @paulnice @pavel-shirshov