Skip to content

Script for updating layers. #25

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
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

@akshayb2022 akshayb2022 commented Apr 29, 2022

./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
password = sys.argv[3]
layer_file = sys.argv[4]
xml_file = sys.argv[5]

Copy link
Contributor

@lpasquali lpasquali May 2, 2022

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

lpasquali commented May 2, 2022

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

@akshayb2022
Copy link
Author

akshayb2022 commented May 3, 2022

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

akshayb2022 commented May 13, 2022

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

randomorder commented May 16, 2022

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