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

Modify script for deployment #398

Merged
merged 7 commits into from
Mar 12, 2022

Conversation

arturtamborski
Copy link
Member

Story / Bug id:

#385

Description:

I modified the deploy script to be more resource agnostic :)

Migrations:

N/A

New imports / dependencies:

N/A

What tests do I need to run to validate this change:

N/A

@arturtamborski arturtamborski marked this pull request as ready for review May 8, 2021 22:29
@arturtamborski
Copy link
Member Author

Please review :)

We can also merge this because it won't affect currently rolled out deployment to staging, but it's still not finished.

  • the actual production environment needs to exist :)
  • aws access key/secret keys need to be encrypted with travis cli by @magul
  • these encrypted secrets need to be placed in this PR

So until that we're kinda blocked, but the PR can be merged.

@arturtamborski arturtamborski requested a review from magul May 9, 2021 10:09
.travis.yml Outdated
Comment on lines 18 to 21
# PRODUCTION_AWS_ACCESS_KEY_ID
#- secure: "TODO REPLACE ME WITH ENCRYPTED VALUES"
# PRODUCTION_AWS_SECRET_ACCESS_KEY
#- secure: "TODO REPLACE ME WITH ENCRYPTED VALUES"
Copy link
Member Author

Choose a reason for hiding this comment

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

waiting for output from CodeForPoznan/infrastructure#84

@OtisRed
Copy link
Contributor

OtisRed commented May 12, 2021

@arturtamborski we will discuss this next week :)

Copy link
Contributor

@OtisRed OtisRed left a comment

Choose a reason for hiding this comment

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

Merging procedure
I leave tactical approve so you can merge it when you see fit.

Contents of the review
I don't follow everything but at least I get some idea what happens here. Left few laic comments but maybe there's something in them - I will let you judge them without remorse :D

Boto3 pivot
Not sure how potential pivot towards AWS API might change this so we shall definitely meet with @w1stler @stanislawK and @jacekkalbarczyk to discuss this.

.travis.yml Outdated
skip_cleanup: true
script: bash deploy.sh production
on:
tags: true
Copy link
Contributor

Choose a reason for hiding this comment

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

This is how we going to control releases, isn't it?


echo "bundle application"
(cd backend && pipenv run pip install -r <(pipenv lock -r) --target ../packages)
(cd backend && pipenv run pip install -r <(pipenv lock -r) --target ../packages)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this second whitespace is here? Mistake or on purpose?

Copy link
Member Author

@arturtamborski arturtamborski Mar 12, 2022

Choose a reason for hiding this comment

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

just to make it easier to the eye - I like aligned lines, so it' easier to compare visually differences between two similar ones.

There's a lot of similar one-liners in the script as you can see, and the differences are mostly tiny, so by aligning the characters in key points (like here, on operator '&') I hope to make it more clear which bits are different.

Comment on lines +3 to +4


Copy link
Contributor

Choose a reason for hiding this comment

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

In some cases you use singular and in some double breaklines and I presume you did it on purpose. If it matters for some reason maybe it would be worth to leave //some kind of docstring?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's just a visual separation of bits in script file. Most of the actions in the script are doubled (one for frontend and then right after one for backend, or one for api lambda and right after one for migration lambda) - so the double newline is just to visually help recognize particular 'steps' in script.

@w1stler
Copy link
Member

w1stler commented Feb 10, 2022

@arturtamborski conflict. :) If we can push this through, I will be happy to assist!

Comment on lines -26 to -29
# AWS_ACCESS_KEY_ID
- secure: "lP1qLhUyJgoJlDqva3dYq3jQh8j1jSXJx19Lq4oNwMLxk4d567LAA/AZHBj8lRqrwW/tjiONb5o1V15+9ogfoK7Y8ika7F3YZkUQ5k/OOliQzDqditSWjYCv3Rx6fc1D5UGQLF1MYYjGctDxiRHmPWb2WfIoqUdrcN4GAbXBKA/EGn8Y680JZm6XZkXFc2PjqoILPwJ+ZIWqAIELibnpHPnRDHD28a3uXjWa1Syehctj2f44wxRNo1hJIQp2mlO9BFS2LrCrhUD2OgeC8gy/j5itzNcDrRwnwZmeZ92YdSsNGGark3K3zPyl5em5v7zlJB/6f/6Jf1IIU0hpc1QAUwZjPd7rRBeYR2U8vUBdYrBlhKFwAJjQjGj4Xm0dgIjcFiANtfppxMt7cFg/c9AUkI+jLGdqfGxkVYKtvYAYWC6QEDikKFwz8eN94zfmkMRkXcniSZPkXAuwLEl9ozz+QAzzNs8BzxzMV4pFB+N/of88B91FyWCchxzSftbrgc8wFmMFZGwzfStgbDU4RX3sGeY7af7rF6uCYS6eHEVwllvJefyNkjUIR8x7wmCXaPjHnKgRzuYrcLtTNUxuc0LMfLCxqVCkGXxflQNIY7Zd59SWukJyloZbUZhxYuoX7YTwVHynNrx+pxvPBP/0QaSnuT8RBZOxmDf1HqT1ShvQgsg="
# AWS_SECRET_ACCESS_KEY
- secure: "JggIiR6ZzXMoK7uCxMgB4nkPzWl7aYXTvIUlkjDdt0OHHySMAEv1D6m45npjCYTqSXwiEuNSplRCMvK72Br1PfWl6hu3d+Nr6DxDTjVu0Xs2jW55h5+A+VNS/Df7Rb2cQ7Ei9e+KI2yoD5oF4ayp62P0RXu060f/9jVnZNFHmR0yJ97eZLcSuY2nmxfQXK/ADnhKIOg0wk2MEnKPlVjzfFBFhvM7h+fmWGaD02QqBfijfnbH6vRdEWICvdkY3eE0Ah7fnETEYMbGVdzGFBouFx66BRKcBFZPbkRtfDNbkIjiClQjWa/HpeXjNxSfgdVrse826qBUN2FrUMR1lxUq4lzSJqNBEQq12if19RFaI12grLo7zfgDIUTOCgXtR+9hGrFredweE7E+q4SmmeFKrzI3fElnEq3PjCzOMMYkF0u3Fvhm9yncZ52SiZYovF4ws6FNxlNud6t6u4jgvphr6fUAHq94g/lLfPgxM/LEIfCaXYHAQ1bKH9MGdqP8zTmQC0OQ2QcpUFl961fiUowIawGrUnkddQGB1AsVlIYehMhVYIhvV7DI9X9ZpYbbADS9WKy2FTzD2KezuJrmZHUumTVT22ZMGai0wP/deLaNEklT6p5qgid4d+ifSvOwaBSTghqpktE6/DbkcgAx4I8o8RkGsl6kucdy8USCRYTHWAE="
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm getting rid of these secret variables from the file, because it's real PITA to change them and there's a new, easier way to do it trough Travis browser UI :) So I moved the access tokens there.

@arturtamborski arturtamborski merged commit 916c2be into CodeForPoznan:master Mar 12, 2022
@arturtamborski arturtamborski deleted the 385-prod-deploy branch March 12, 2022 00:15
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