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

Brianna/step-one #15

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Brianna/step-one #15

wants to merge 8 commits into from

Conversation

BCerki
Copy link

@BCerki BCerki commented Feb 23, 2022

No description provided.

@BCerki BCerki reopened this Feb 23, 2022
task text not null,
completed boolean not null default false,
date_created timestamptz not null default now(),
date_updated timestamptz not null default now()
Copy link
Author

Choose a reason for hiding this comment

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

I originally forgot to include default now() and to get it in, I had to revert and then redeploy. Is that the right way to do it?

Choose a reason for hiding this comment

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

In your development environment, yes. For some changes the way might even be to drop and recreate your db.

If this happens in a prod environment, you will have to create a new change which alters the column. The changes that are already deployed should be immutable. You can't revert and redeploy there otherwise you're dropping the tables and loosing your data

task text not null,
completed boolean not null default false,
date_created timestamptz not null default now(),
date_updated timestamptz not null default now()

Choose a reason for hiding this comment

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

In your development environment, yes. For some changes the way might even be to drop and recreate your db.

If this happens in a prod environment, you will have to create a new change which alters the column. The changes that are already deployed should be immutable. You can't revert and redeploy there otherwise you're dropping the tables and loosing your data

%syntax-version=1.0.0
%project=todo_app

todo_appschema 2022-02-23T22:43:21Z Brianna Cerkiewicz <briannacerkiewicz@pop-os> # Add schema for all todo objects.

Choose a reason for hiding this comment

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

remember to configure sqitch to have your work email in your profile

create or replace function todo_app.insert_todo(
task text,
completed boolean
) returns void language sql security definer as $$

Choose a reason for hiding this comment

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

defining a function with security definer is to be avoided if possible. A security definer function will be executed with the privileges of the user that creates it.


BEGIN;

create or replace function todo_app.insert_todo(

Choose a reason for hiding this comment

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

In most cases you won't need to create functions to insert your data. postgraphile will automatically generate graphql mutations to insert, update and delete data.

Comment on lines +6 to +11
BEGIN;

select todo_app.insert_todo('task not done', false);
select todo_app.insert_todo('task done', true);

COMMIT;

Choose a reason for hiding this comment

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

for seed data, we've been following the following pattern in our project:

  • if the data is actually "seed" data, i.e. the initial data which can later be modified by our users, then a sqitch migration, like this is good
  • if the data is hardcoded data, which users are not expected to change, but developers may want to change between releases, we use a different set of sql scripts containing idempotent changes (example from the ciip repo).

@BCerki BCerki changed the title Brianna step 1 Brianna/step-one Mar 3, 2022
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.

2 participants