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

Script for updating layers. #25

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Script for updating layers. #25

wants to merge 4 commits into from

Conversation

akshayb2022
Copy link

./update_tile_layers.py geoserver_url geoserver_username geoserver_pass ./layers.txt tl_config_template.xml

@akshayb2022 akshayb2022 requested a review from randomorder April 29, 2022 08:00
@akshayb2022 akshayb2022 self-assigned this Apr 29, 2022
@randomorder randomorder requested review from lpasquali and removed request for randomorder April 29, 2022 08:05
@randomorder
Copy link
Member

randomorder commented Apr 29, 2022 •
edited
Loading

can you test / review this @lpasquali ?

@randomorder randomorder mentioned this pull request Apr 29, 2022
Closed
lpasquali
lpasquali suggested changes May 2, 2022
View reviewed changes
Copy link
Contributor

@lpasquali lpasquali left a comment

Choose a reason for hiding this comment

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

this script has nothing to do with performances, it should be moved under utils.

lpasquali
lpasquali suggested changes May 2, 2022
View reviewed changes
Copy link
Contributor

@lpasquali lpasquali left a comment

Choose a reason for hiding this comment

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

Please add requirements.txt for python modules used

lpasquali
lpasquali reviewed May 2, 2022
View reviewed changes
performance_tests/update_layers/update_xml_file.py Outdated
password = sys.argv[3]
layer_file = sys.argv[4]
xml_file = sys.argv[5]

Copy link
Contributor

Choose a reason for hiding this comment

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

please add checks to validate presence of correct arguments and, if something is not right, print the usage you put on top.

lpasquali
lpasquali suggested changes May 2, 2022
View reviewed changes
Copy link
Contributor

@lpasquali lpasquali left a comment

Choose a reason for hiding this comment

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

please validate arguments and print an usage in case something is missing

@lpasquali
Copy link
Contributor

Hello @akshayb2022, I reviewed you code, please check my comments and update it with changes requested, many thanks in advance!

@akshayb2022
Copy link
Author

Hello @akshayb2022, I reviewed you code, please check my comments and update it with changes requested, many thanks in advance!

Hello @lpasquali
Thanks for reviewing the code.
I will do the mentioned changes and update you.

@randomorder randomorder requested a review from lpasquali May 4, 2022 08:20
@lpasquali
Copy link
Contributor

lpasquali commented May 9, 2022 •
edited
Loading

hello @akshayb2022 I see you made most of changes requested.
Please do as well https://github.com/geosolutions-it/scripts/pull/25/files/e9e4bc3a8864dd1763b5e85fc225c3f83b14c9c5#r862699591
and if you can add a bried README.md with same content of the usage it would be super.
Many thanks in advance.

@akshayb2022
Copy link
Author

hello @akshayb2022 I see you made most of changes requested. Please do as well https://github.com/geosolutions-it/scripts/pull/25/files/e9e4bc3a8864dd1763b5e85fc225c3f83b14c9c5#r862699591 and if you can add a bried README.md with same content of the usage it would be super. Many thanks in advance.

Done, please check

@randomorder
Copy link
Member

are we done here @lpasquali ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpasquali lpasquali Awaiting requested review from lpasquali

Assignees

@akshayb2022 akshayb2022

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@akshayb2022 @randomorder @lpasquali