Skip to content

root: replace builtin psycopg libpq binary implementation with distro…#6448

Merged
BeryJu merged 1 commit intomainfrom
psycopg-c
Aug 2, 2023
Merged

root: replace builtin psycopg libpq binary implementation with distro…#6448
BeryJu merged 1 commit intomainfrom
psycopg-c

Conversation

@rissson
Copy link
Member

@rissson rissson commented Aug 1, 2023 •
edited
Loading

… provided one

Details

As discussed previously.

https://www.psycopg.org/psycopg3/docs/basic/install.html#local-installation recommends using the C implementation when possible.

REPLACE ME

Checklist

  • Local tests pass (ak test authentik/)
  • The code has been formatted (make lint-fix)

If an API change has been made

  • The API schema has been updated (make gen-build)

If changes to the frontend have been made

  • The code has been formatted (make web)
  • The translation files have been updated (make i18n-extract)

If applicable

  • The documentation has been updated
  • The documentation has been formatted (make website)

@rissson rissson requested a review from a team as a code owner August 1, 2023 20:33
@rissson rissson requested review from kensternberg-authentik and removed request for a team August 1, 2023 20:33
@netlify
Copy link

netlify bot commented Aug 1, 2023 •
edited
Loading

✅ Deploy Preview for authentik-storybook canceled.

Name Link
🔨 Latest commit 70dfd86
🔍 Latest deploy log https://app.netlify.com/sites/authentik-storybook/deploys/64c970e282c86900074f260f

@rissson rissson requested review from a team and BeryJu and removed request for kensternberg-authentik August 1, 2023 20:33
@rissson
Copy link
Member Author

rissson commented Aug 1, 2023 •
edited
Loading

For reference and comparison:
CI on main: https://github.com/goauthentik/authentik/actions/runs/5730041231/job/15527967561
CI on branch: https://github.com/goauthentik/authentik/actions/runs/5731746647/job/15533304446?pr=6448

@rissson
Copy link
Member Author

rissson commented Aug 1, 2023

Setting up the dev env in the CI takes about the same time (2-3 minutes)

@rissson
root: replace builtin psycopg libpq binary implementation with distro…
70dfd86 
… provided one

Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
@codecov
Copy link

codecov bot commented Aug 1, 2023 •
edited
Loading

Codecov Report

Patch coverage has no change and project coverage change: +44.61% 🎉

Comparison is base (cc6824f) 47.84% compared to head (70dfd86) 92.45%.

Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #6448       +/-   ##
===========================================
+ Coverage   47.84%   92.45%   +44.61%     
===========================================
  Files         561      561               
  Lines       27075    27075               
===========================================
+ Hits        12951    25029    +12078     
+ Misses      14124     2046    -12078
Flag Coverage Δ
e2e 51.59% <ø> (+3.76%) ⬆️
integration 26.56% <ø> (?)
unit 89.28% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 406 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

BeryJu
BeryJu reviewed Aug 1, 2023
View reviewed changes
Dockerfile
apt-get install -y --no-install-recommends runit && \
pip install --no-cache-dir -r /requirements.txt && \
apt-get remove --purge -y build-essential pkg-config libxmlsec1-dev && \
apt-get remove --purge -y build-essential pkg-config libxmlsec1-dev libpq-dev python3-dev && \
Copy link
Member

@BeryJu BeryJu Aug 1, 2023

Choose a reason for hiding this comment

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

wouldn't python3-dev interfere with the docker python install? Since I assume that one is not installed via dpkg

Copy link
Member Author

@rissson rissson Aug 1, 2023

Choose a reason for hiding this comment

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

After checking what python3-dev and dependencies install vs what's already bundled in the image, I don't think it will.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023 •
edited
Loading

authentik PR Installation instructions

Instructions for docker-compose

Add the following block to your .env file:

AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-psycopg-c-1690924082-70dfd86
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s

For arm64, use these values:

AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-psycopg-c-1690924082-70dfd86-arm64
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s

Afterwards, run the upgrade commands from the latest release notes.

Instructions for Kubernetes

Add the following block to your values.yml file:

authentik:
    outposts:
        container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
image:
    repository: ghcr.io/goauthentik/dev-server
    tag: gh-psycopg-c-1690924082-70dfd86

For arm64, use these values:

authentik:
    outposts:
        container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
image:
    repository: ghcr.io/goauthentik/dev-server
    tag: gh-psycopg-c-1690924082-70dfd86-arm64

Afterwards, run the upgrade commands from the latest release notes.

@rissson
Copy link
Member Author

rissson commented Aug 1, 2023

Docker image size is 1MB heavier with this PR.

@BeryJu
Copy link
Member

BeryJu commented Aug 1, 2023

Docker image size is 1MB heavier with this PR.

I was just about to comment on that, they look the same size here

ghcr.io/goauthentik/dev-server          gh-psycopg-c-1690924082-70dfd86-arm64                   cb2f6d67160f   6 minutes ago   798MB
ghcr.io/goauthentik/dev-server          gh-next-1690912046-cc6824f-arm64                        db264d6c234a   3 hours ago     798MB

@BeryJu BeryJu merged commit 90f4ebf into main Aug 2, 2023
@BeryJu BeryJu deleted the psycopg-c branch August 2, 2023 09:48
@rissson
Copy link
Member Author

rissson commented Aug 2, 2023

I just realized but this may break some development environments if those packages are not installed. I'll put in a follow up PR to update the dev docs

@BeryJu
Copy link
Member

BeryJu commented Aug 2, 2023

I just realized but this may break some development environments if those packages are not installed. I'll put in a follow up PR to update the dev docs

I did wonder about that too, but it installed just fine for me locally without any changes

We do already tell people to install libpq so it should all be fine?

@rissson
Copy link
Member Author

rissson commented Aug 2, 2023

Yep, it will be fine indeed

kensternberg-authentik added a commit that referenced this pull request Aug 7, 2023
@kensternberg-authentik
Merge branch 'main' into application-wizard-2
79e0a32 
* main: (36 commits)
  website/blog: add github user name links (#6468)
  website/developer-docs: add new template for procedures (#6390)
  website/blogs: blog to celebrate hackathon (#6457)
  web/flows: add more stories (#6444)
  web: bump prettier from 3.0.0 to 3.0.1 in /web (#6465)
  core: bump debugpy from 1.6.7 to 1.6.8 (#6458)
  ci: bump peter-evans/create-pull-request from 4 to 5 (#6459)
  web: bump lit from 2.7.6 to 2.8.0 in /web (#6460)
  web: bump @fortawesome/fontawesome-free from 6.4.0 to 6.4.2 in /web (#6461)
  web: bump chart.js from 4.3.2 to 4.3.3 in /web (#6462)
  web: bump @lit-labs/task from 2.1.2 to 3.0.0 in /web (#6463)
  web, website: compress images (#6121)
  core: bump cryptography from 41.0.2 to 41.0.3 (#6456)
  root: replace builtin psycopg libpq binary implementation with distro… (#6448)
  website: fix broken links in NewsBar
  core: bump github.com/getsentry/sentry-go from 0.22.0 to 0.23.0 (#6449)
  core: bump goauthentik.io/api/v3 from 3.2023061.6 to 3.2023061.7 (#6450)
  web: bump pyright from 1.1.319 to 1.1.320 in /web (#6451)
  core: bump ruff from 0.0.281 to 0.0.282 (#6453)
  core: bump golang from 1.20.6-bullseye to 1.20.7-bullseye (#6454)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BeryJu BeryJu BeryJu approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rissson @BeryJu