-
-
Notifications
You must be signed in to change notification settings - Fork 729
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
Creates GH Actions section for sample data #13095
base: master
Are you sure you want to change the base?
Creates GH Actions section for sample data #13095
Conversation
This is intended to be used by testers, to generate sample data, on demand
It's a bit difficult to know whether this will work, as the changes will only be visible after merging. At least I'm not aware of any way to test this, in advance. Also, I'm not sure the script section will do what it should. Some questions:
|
mkdir -p ~/backup/ | ||
pg_dump -h localhost -U ofn_user openfoodnetwork | gzip > ~/backup/staging-`date +%Y%m%d%H%M%S`-before-reset.sql.gz | ||
bundle exec rake db:reset ofn:sample_data | ||
sudo systemctl stop puma sidekiq |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be starting puma/sidekiq again :
sudo systemctl stop puma sidekiq | |
sudo systemctl start puma sidekiq |
@@ -35,5 +35,5 @@ jobs: | |||
sudo systemctl stop puma sidekiq | |||
mkdir -p ~/backup/ | |||
pg_dump -h localhost -U ofn_user openfoodnetwork | gzip > ~/backup/staging-`date +%Y%m%d%H%M%S`-before-reset.sql.gz | |||
bundle exec rake db:reset ofn:sample_data | |||
bundle exec rake db:reset ofn:${{ secrets.sample_data_name }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I am following this, as far as I understand we only have ofn:sample_data
task for sample data ?
What are the expected inputs here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can imagine it might be useful to provide individual scripts for each sample data factory (eg users, enterprises, fees). But that hasn't been done yet, so I guess Filipe was just testing to see what's possible. We can't test it until it's merged so I'm happy to leave this in.
It's worth noting the sample data factories depend on each other (eg sample enterprises are managed by the sample users), so if you execute enterprises before users, the script would probably fail. But as long as the full sample data script is executed first, that should never be a problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- do we really need to reset the DB? Or can we run the sample_data script on top of existing data? It would be best to keep existing data.
It looks like the sample_data script can run on top of existing data. If there is a matching record (eg user with same email, or enterprise with same name), the record will be overwritten with the sample data (enterprise data shown here for example: https://github.com/openfoodfoundation/openfoodnetwork/blob/master/lib/tasks/sample_data/enterprise_factory.rb#L24)
So we can do this without a reset:
bundle exec rake db:reset ofn:${{ secrets.sample_data_name }} | |
bundle exec rake ofn:${{ secrets.sample_data_name }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- DB backup: should we run the command provided on the test instructions from 2072 New sample data for testing #3209 or use the baseline resetting option, as introduced in Add scripts to save and restore baseline data #12947?
Use the new script. Of course, to restore the db would require another GH Action, which you could make after this one succeeds :D
@@ -35,5 +35,5 @@ jobs: | |||
sudo systemctl stop puma sidekiq | |||
mkdir -p ~/backup/ | |||
pg_dump -h localhost -U ofn_user openfoodnetwork | gzip > ~/backup/staging-`date +%Y%m%d%H%M%S`-before-reset.sql.gz | |||
bundle exec rake db:reset ofn:sample_data | |||
bundle exec rake db:reset ofn:${{ secrets.sample_data_name }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can imagine it might be useful to provide individual scripts for each sample data factory (eg users, enterprises, fees). But that hasn't been done yet, so I guess Filipe was just testing to see what's possible. We can't test it until it's merged so I'm happy to leave this in.
It's worth noting the sample data factories depend on each other (eg sample enterprises are managed by the sample users), so if you execute enterprises before users, the script would probably fail. But as long as the full sample data script is executed first, that should never be a problem.
@@ -35,5 +35,5 @@ jobs: | |||
sudo systemctl stop puma sidekiq | |||
mkdir -p ~/backup/ | |||
pg_dump -h localhost -U ofn_user openfoodnetwork | gzip > ~/backup/staging-`date +%Y%m%d%H%M%S`-before-reset.sql.gz | |||
bundle exec rake db:reset ofn:sample_data | |||
bundle exec rake db:reset ofn:${{ secrets.sample_data_name }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- do we really need to reset the DB? Or can we run the sample_data script on top of existing data? It would be best to keep existing data.
It looks like the sample_data script can run on top of existing data. If there is a matching record (eg user with same email, or enterprise with same name), the record will be overwritten with the sample data (enterprise data shown here for example: https://github.com/openfoodfoundation/openfoodnetwork/blob/master/lib/tasks/sample_data/enterprise_factory.rb#L24)
So we can do this without a reset:
bundle exec rake db:reset ofn:${{ secrets.sample_data_name }} | |
bundle exec rake ofn:${{ secrets.sample_data_name }} |
inputs: | ||
server: | ||
description: "Staging Server" | ||
type: choice | ||
required: true | ||
options: | ||
- staging.openfoodnetwork.org.uk | ||
- staging.openfoodnetwork.org.au | ||
- staging.coopcircuits.fr | ||
sample_data_name: | ||
description: "Sample Data Name" | ||
type: string | ||
required: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really nice to have some input for an action.
- name: Run sample data script | ||
if: success() | ||
run: | | ||
sudo systemctl stop puma sidekiq | ||
~/apps/openfoodnetwork/current/script/ci/save-staging-baseline.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry but this doesn't work.
This script is executed on the Github Actions machine. It's not executed on one of the staging servers. In theory, we could execute commands via ssh
on the staging servers but at the moment they don't allow that. We would need to authorize an SSH key stored in Github to perform these commands. And I'm not sure we want that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😅 eep I missed that! We do currently execute commands remotely with ansible when deploying to staging, so it wouldn't be too much of stretch to do the same here.
What we need is for this script to be saved on the server. Then it's pretty trivial to log in and run it. eg:
ssh [email protected]
script/load_sample_data
If we have that, then setting up this remote trigger doesn't seem worth the effort. But if it makes it easier for non-technical testers (ie Rachel) then perhaps it is worth the effort. Maybe we need to discuss at delivery-circle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script is executed on the Github Actions machine
Uff, I obviously I missed that as well. Thanks for pointing that out @mkllnk.
What we need is for this script to be saved on the server. Then it's pretty trivial to log in and run it.
Ok, I'll have a second look at this @dacook . I'm not sure though I fully grasp the complexity here - I'm looking out for this Slack thread, and then proceed. Thanks for your help and reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I wasn't very prescriptive. I'm not sure if Maikel has concerns about this approach.
I suggest the next step is to create a new script in ofn-install. I had a look.. it's not easy to explain.. I'll try it..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maikel what do you think of this? openfoodfoundation/ofn-install#994
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, all possible. I was just cautious because it's not quick and easy. But if there's enough appetite for this feature then sure we can do it.
Adding the blocked label till we figure out the best way forward. Discussion ongoing here. |
What? Why?
Introduces a section on GH-Actions to provide the possibility to manage creation of sample data.
Mockup:
Marked in yellow: section on branch/tag to be removed. Not sure how to do this.
What should we test?
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.
Dependencies
Documentation updates
We'll need to add this to the testers handbook.