dev-contrib

[fallen patrol]

^

It's not, it just doesn't show

[static canyon]

Yeah, it's because you don't have permissions then

I guess it's a private group and you need to part of the org

[green oriole]

Huh, can one of the owners (CC @patent pivot, @hardy gorge, @crude gyro) change the visibility of the devops team on github to public so review requests properly show up for people outside of the org?

[clever wraith]

I've discovered something that could be really useful for threads, if a bot sends a message and then edit in a mod ping, all the mods get added to the thread without getting any actual ping

You could use that if you wish to have the mods auto join threads, I've seen some communities do that, it could be a good practice

[short snow]

" and then edit in a mod ping" uhh I am not exactly clear on what you mean by this?

[vocal prairie]

Just edit from "hi I will edit in a mod ping here:" to "hi I will edit in a mod ping here: @mods", since edits don't ping

[gritty wind]

Why would we add mods to threads by default

Seems like terrible UX for mods

[short snow]

ah 👍

[cold island]

Isn't there an endpoint to join someone to a thread?

[clever wraith]

Not that I'm aware of

[cold island]

I remember @vale ibex playing with it recently

[clever wraith]

The idea would be so mods can watch over threads like they would with normal channels

[gritty wind]

Sadly just pings

[gritty wind]

Afaik most mods don't just go from channel to channel looking for trouble

On the other hand, most mods use the server like normal users would

Meaning, they have channels they like to visit and see, and organize the server in some way

We already have experience with auto opt-in (modmail), and tbh, it's shit

90% of the time I don't want to deal with modmail, but I'm still forced to see the channels, and deal with pings

[short snow]

Could https://github.com/python-discord/bot/pull/1634 get another review :D

[austere hornet]

Can't you ping the Owners role? Just a suggestion in the future. I mean, doesn't matter really, I personally find it easier to ping a group of people by role

[green oriole]

Nah we can't

It isn't pingeable

[austere hornet]

Ohh

I thought it was

But ok

[patent pivot]

unless someone else got to it this is already done

it’s not in the audit log

[short snow]

nope

still not found

[green oriole]

How weird

Why doesn't it show the review properly

[short snow]

🤷‍♂️

[green oriole]

https://github.blog/changelog/2021-11-10-require-pull-requests-without-requiring-reviews/ also this is interesting

[vale ibex]

Yes there is, but it's per user request, rather than roles

It also pings that user

[cold island]

ah that sucks

[shy juniper]

помогите пж с програмированием мне срочно

[tawdry vapor]

This is an English community. Please try your best to write in English.

[shy juniper]

(ok

[tawdry vapor]

If you need help with Python you can read here for how to ask #❓｜how-to-get-help

[dim pelican]

Sorta out there, but what do you think about sir-lancebot#834 regarding commit messages? (cross-posting from #community-meta )

[dusky shore]

GitHub

Add .gitmessage to allow for consistent commit messages

[short snow]

https://paste.fuelrats.com/otuxerolez.php this is what I use, cc @green oriole

[green oriole]

I like the general structure but I personally hate the conventional commit stuff, I find it super useless

[gritty wind]

+1 to that

The idea of giving people who don’t know what a commit should be a general idea to follow sounds fine

But like I’ll be promptly disabling it locally

I also don’t want us to enforce a general commit format

[brisk brook]

I really dislike the conventional commits thing. It always gets past 72 characters

[dim pelican]

This is really where I wanted to have the discussion, is it even worthwhile?

[thorny obsidian]

it's just 72 chars per line, right? I just split it onto multiple lines

[dim pelican]

50 for subject, 72 after that I think

[gritty wind]

72 per line, except if it’s the subject which some people choose to do 50, but there’s no point since 72 is still visible

[thorny obsidian]

Is the 72 visible for the commit title? I thought some things only displayed 50 for the title

[brisk brook]

Discord.js uses conventional commits (with the type stuff) and I've had to use it for a PR. Hated it in all honesty

[gritty wind]

I only really look at GitHub, terminal, and my ide. They all show it

Don’t know where else people are looking. Email? That’s longer I think lol

[green oriole]

Github is 50 I think?

[brisk brook]

50 characters is the community-wide agreement, like 79 characters for a Python file

[brazen charm]

github does 72

[brisk brook]

If it exceeds 72 you get this:

[green oriole]

The subject line limit is like whatever

You have wrap around nowadays, crazy stuff

[thorny obsidian]

I prefer the line limits (whether it's 50 or 7e). I find it encourages brevity and conciseness. It's also nice to just look at commit titles without it going "..." and having to look into the commit body itself

Plus for git log it doesn't always support word wrap and I appreciate when people keep to the line length limit

[dim pelican]

Like showing the 50 character limit, a small reminder / prompt to use imperative language, then a 72 character mark

[gritty wind]

Tbh I have a very low opinion of the standardization of commits (besides the char limit)

As long as your commit conveys what you want to say

It’s a-ok with me

[thorny obsidian]

I think it would be nice to have in lancebot, just because we get a lot of new contribs who don't know how to write a commit and this would provide some nice guidance

[gritty wind]

Yeah, I wouldn’t mind having it as a guide, but not necessarily enforced

[thorny obsidian]

because we do get a lot of commits that are like "fix stuff from review" which is not exactly helpful

[gritty wind]

Yeah haha

Would be nice getting rid of those

[dim pelican]

I'm guilty of the above

[gritty wind]

We all are, ya don’t know

What you don’t know

[brisk brook]

Why waste space to fit in the type thing? I remember fighting the pre-commit hooks for so long for that damm discord.js PR lol. I think I wanted to do like refactor(InteractionCollector): Only store Ids of objects on the InteractionCollector

[dim pelican]

I'm fine with leaving it out of the git message, I just got into the habit of tagging my commits recently

[brisk brook]

I am guilty of this 😬. GitHub Desktop doesn't wrap the body so it's really hard to cut it off myself..

[thorny obsidian]

lol I always manually wrap mine since I commit from the editor/terminal

[patent pivot]

magit is love, magit is life

[green oriole]

Yeah, I mean for the body body, not the title

[thorny obsidian]

I mean, I still prefer line limits for the body. It helps with readability and not everything has nice word wrap

[brisk brook]

I've been considering doing this. Do all staging in GitHub and then commit in there because I find it better than this small area:

[trail pilot]

Same, sometimes after coding for a while I forgot to make small commits so I forgot everything I added

[static canyon]

Is there a particular reason as to why sqlalchemy isn't including in the poetry.lock on the API repo and so doesn't install?

Nvm, it is in the poetry.lock, it just didn't install when I did poetry install 🤔

A simple pip install sqlalchemy==1.4.18 works 🤷

🤷

Okay wtf

Now django didn't install when I did poetry install on the site repo

The logs of the poetry install says it's installing it but then it doesn't show in pip freeze (or pip list)

So... it says it's installing but then never installs?

C:\Users\tizzy\site>poetry install
Installing dependencies from lock file

No dependencies to install or update
```but the libraries aren't all installed

I'm so confused

[gritty wind]

Try poetry debug and poetry run pip list

[static canyon]

C:\Users\tizzy\site>poetry debug

Poetry
Version: 1.1.7
Python:  3.9.5

Virtualenv
Python:         3.9.5
Implementation: CPython
Path:           C:\Users\tizzy\AppData\Local\pypoetry\Cache\virtualenvs\site--h5SVkFS-py3.9
Valid:          True

System
Platform: win32
OS:       nt
Python:   C:\Users\tizzy\AppData\Local\Programs\Python\Python39

[static canyon]

Everything seems to show in poetry run pip list but not pip list so I guess pip is pointing to something else for some reason 🤷

[gritty wind]

Pip -VVVVV

In both environments

The more Vs, the better

[static canyon]

C:\Users\tizzy\site>pip -V
pip 21.3.1 from c:\users\tizzy\appdata\local\programs\python\python39\lib\site-packages\pip (python 3.9)

C:\Users\tizzy\site>poetry run pip -V
pip 21.3 from C:\Users\tizzy\AppData\Local\pypoetry\Cache\virtualenvs\site--h5SVkFS-py3.9\lib\site-packages\pip (python 3.9)
```yep, different

[gritty wind]

Poetry shell to activate

[static canyon]

What do you mean?

[gritty wind]

The command poetry shell will drop you into an activated environment with the correct venv

[static canyon]

Right

PyCharm still gives lint-type errors saying it's not installed though

Do I just have to live with that? @gritty wind

[vocal prairie]

I believe there is a poetry plugin, do you already have that? If so, I'm not sure.

[gritty wind]

Did you select it in pycharm

Poetry plug-in is mostly only useful for running poetry install, not strictly necessary

You’d just add the poetry venv like you’d add any other venv

[static canyon]

Hmm

Poetry isn't showing as a thing

[gritty wind]

Just virtualenv

Existing to be specific

[static canyon]

I mean for the "Python Interpreter"

[gritty wind]

https://www.jetbrains.com/help/pycharm/creating-virtual-environment.html#existing-environment

The link you put into this is the one you got from poetry debug

[static canyon]

For bot and lancebot it shows a poetry thing

[gritty wind]

That’s not built into pycharm, it’s a separate plug-in, but it’ll only create a new virtualenv and basically do this ^ afterwards

You basically manually did what the plug-in does when you ran poetry install

[static canyon]

I'm so confused

[gritty wind]

Alright, so to clarify something, just forget poetry as a concept. What’s relevant right now is that ‘poetry install’ is equivalent to doing a pip install into a virtualenv. If you did that, and wanted to add it to pycharm, you’d add the virtualenv.

Now the plug-in goes a step further, basically creating the venv and running “pip install” (sticking with the simplification)

You’ve already done the part in the second paragraph by hand

[static canyon]

So... what do I need to do to make PyCharm recognise that stuff is installed?

[gritty wind]

^ this

[static canyon]

That's not a thing for me

It says to press options that aren't showing

[gritty wind]

I wonder if the guide is out of date

SS?

[static canyon]

Okay so ignore that comment

I just don't know which of these to do for part 3

[gritty wind]

Click the plus at the top -> virtualenv -> existing

[static canyon]

And what's the interpreter?

[gritty wind]

The one you got from poetry debug

So

[static canyon]

C:\Users\tizzy\AppData\Local\pypoetry\Cache\virtualenvs\site--h5SVkFS-py3.9?

[gritty wind]

This

Yeah

Err sorry

[static canyon]

That's a folder

[gritty wind]

You need to select the python executable inside it

So the path at the bottom of debug

[static canyon]

python or pythonw?

[gritty wind]

Just python

[static canyon]

C:\Users\tizzy\AppData\Local\pypoetry\Cache\virtualenvs\site--h5SVkFS-py3.9\Scripts\python.exe?

[gritty wind]

Yeah that should do it

[static canyon]

Right, okay

Let's see

That worked, thank you very much 👍

[gritty wind]

Yup

[molten perch]

Hey, so I've been thinking about this async SQLAlchemy and sync SQLAlchemy, and I find it a bit dubious. (API ofc. 😄 )
Mostly, because of model validations eg.: https://github.com/python-discord/api/blob/main/api/core/database/models/api/bot/user.py#L60:L69, how would we circumvent this?

[stable mountain]

api/core/database/models/api/bot/user.py lines 60 to 69

@validates("roles")
def validate_roles(self, _key: str, roles: list[int]) -> Union[list[int], NoReturn]:
    """Raise ValueError if the provided role(s) is negative or non-existent."""
    session = SessionLocal()
    for role in roles:
        if role < 0:
            raise ValueError("Role IDs cannot be negative")
        if not session.query(Role).filter_by(id=role).first():
            raise ValueError(f"Role with ID {role} does not exist")
    return roles```

[static canyon]

So I'm about to do this, is there anything I should do first?

I've just addedpy dm_sent = models.BooleanField( null=True, help_text="Whether a DM was sent to the user when infraction was applied." )(site repo)

[molten perch]

Well, I believe it should be fine.

[static canyon]

Okay, cool

Let's see

[molten perch]

Although, I'm not an expert on Django ORM.

[static canyon]

django.core.exceptions.ImproperlyConfigured: Set the SECRET_KEY environment variable

What do I set it to (and how)?

[molten perch]

I guess, you are not using docker, are you?

[static canyon]

What do you mean?

Ah no

I just do manage.py xxxx in PyCharm's terminal

I can use docker if I'm supposed to (and you tell me how)

[molten perch]

No, actually. The thing is that, if you don't use Docker, you've got to set the environment variables.
See: https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/site/#2-environment-variables
(I believe you have already set the DB URL)

And that's where you have to set the SECRET_KEY

[static canyon]

Hmm

Would it be easier to use docker then?

I'm kinda confused

[molten perch]

I mean it would be easier 😄
I started off with Docker in the first place, but I figured that if you haven't set it up already, you would want to stick with not using Docker.

[static canyon]

I've already set up Docker for bot/sir-lancebot

I assume it's the same 🤷

[static canyon]

So do I do docker-compose up postgres?

[molten perch]

If you want to run only the database, then yes. But the SECRET_KEY won't be set.
If you want to run everything in Docker you issue: docker-compose up and it's gonna run the site and the database as well, with all the environment variables pre-set

[static canyon]

Right, okay

I guess I'll do the latter then

Where do I do the manage.py xxxx by the way?

[static canyon]

I'm still getting this @molten perch

I'm probably just running in the wrong place

[tawdry vapor]

Are you trying to use manage.py for something other than running the server?

[static canyon]

I'm trying to get the auto-generated migration thing for when you edit the database schema

[tawdry vapor]

makemigrations?

[static canyon]

Trying to get this basically: https://github.com/python-discord/site/pull/618/files#diff-e942db86bcaa00ecf50ca948b4a8f42ae16de841f5370a6e2cb78384349632af

[static canyon]

manage.py makemigrations gives the above error about the SECRET_KEY

[tawdry vapor]

You're trying to run that through Docker?

[static canyon]

I think so?

If I understand correctly, D0rs4n said running on Docker would mean I don't have to worry about setting the envs

[tawdry vapor]

If you're relying on Docker Compose for the dev environment rather than setting one up on the host, then yeah

I suppose one easy way to run it would be to edit the docker-compose file to change the command it calls

You'll see it normally shows command: ["run", "--debug"]

But you could change that to command: ["makemigrations"].

And since Docker Compose is configured to mount your host's directory, those migrations should actually end up on your actual host file system.

[molten perch]

I'm not sure it would work. The file system is Read Only, isn't it?

[tawdry vapor]

Oh, true

[static canyon]

version: "3.6"
services:
  postgres:
    ...
    environment:
      POSTGRES_DB: pysite
      POSTGRES_PASSWORD: pysite
      POSTGRES_USER: pysite
    ...

  web:
    ...
    environment:
      DATABASE_URL: postgres://pysite:pysite@postgres:5432/pysite
      METRICITY_DB_URL: postgres://pysite:pysite@postgres:5432/metricity
      SECRET_KEY: suitable-for-development-only
      STATIC_ROOT: /var/www/static
      DEBUG: 1

volumes:
  staticfiles:
```I want to somehow configure the environment stuff then I guess

[tawdry vapor]

Then that would mean it's not possible to do this through Docker Compose? Unless you also change the mount.

[molten perch]

Then, you could run the database in docker, and run the web app locally, and just set the env variables with an .env file?

[tawdry vapor]

That's what I do

Well, I use docker compose to run it, but I also have all dependencies and env vars set up locally in case I need to do something other than running the site, or I need to attach a debugger while running.

[static canyon]

So I just need to do```

.env file

POSTGRES_DB=pysite
POSTGRES_PASSWORD=pysite
POSTGRES_USER=pysite
DATABASE_URL: postgres://pysite:pysite@postgres:5432/pysite
METRICITY_DB_URL: postgres://pysite:pysite@postgres:5432/metricity
SECRET_KEY: suitable-for-development-only
STATIC_ROOT: /var/www/static
DEBUG: 1```?

[molten perch]

I believe, so.

[tawdry vapor]

Not exactly

[molten perch]

Then, you can run via poetry.

[tawdry vapor]

You probably don't want STATIC_ROOT to be going to /var/www on your host

[molten perch]

Oh, yeah follow this: https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/site/#2-environment-variables
It has all the variables you need.

[tawdry vapor]

Also, the database connections would be different since you wouldn't be able to interact with docker's internal network

The DB would be exposed on localhost:7777

[vale ibex]

For patsy I'm mounted a volume in docker compose to the migration path

And the did docker-compose exec web makemigrations

Well I used alembic, but same concept

[vocal prairie]

.bm dawn you've been trying to figure out this command for months, you're an idiot

[vale ibex]

I'm on mobile so can't link the exact thing you need to do

But take a look at the patsy docker compose

I also have a script in the pyproject toml

Would just need changing from alembic to django

[static canyon]

I'm starting to get lost again

[molten perch]

I think the easiest way is to go with

DATABASE_URL=postgres://pysite:pysite@localhost:7777/pysite
METRICITY_DB_URL=postgres://pysite:pysite@localhost:7777/metricity
DEBUG=1
SECRET_KEY=suitable-for-development-only
STATIC_ROOT=staticfiles```
would be that in your .env, wouldn't it?

[vale ibex]

Yea if you want to run django locally

If you use a docker compose volume, you don't need to setup an env

[molten perch]

Yeah, I think it's easier for now.

[static canyon]

I don't particularly get this

Isn't it already localhost?

[vale ibex]

It's literally just add one line in the compose file

And we can avoid having to set env up

[static canyon]

🤔

[tawdry vapor]

Is it though? We have multiple apps each with their own migration folder

Well, IDK how many of them actually need migrations

[vale ibex]

Are migrations per path?

[tawdry vapor]

Yeah I think so

[vale ibex]

Ahh right, then it would be one per migration path then

[tawdry vapor]

Yeah you're right. Might have been an oversight when the guide was being rewritten.

[brazen charm]

I used the verbose output and just copied it to the file to go around the read only limitation

[vale ibex]

I might take a look at it tomorrow to see if we can ease generating migrations

For full docker setups that is

[tawdry vapor]

while you're at it, add a volume for the pgdata since we agreed to do that

[vale ibex]

Sure

[static canyon]

Right, okay. That was greatly confusing me lol

[vale ibex]

!remind 14h

[stable mountain]

Missing required argument

content

[vale ibex]

Ugh

[vale ibex]

!remind 14h g

[stable mountain]

Yep.

Your reminder will arrive on <t:1636632418:F>!

ID: 3454

[static canyon]

C:\Users\tizzy\site>manage.py makemigrations
Migrations for 'api':
  pydis_site\apps\api\migrations\0075_infraction_dm_sent.py
    - Add field dm_sent to infraction
```I guess this means it worked?

The generated file doesn't comply to the 120 char limit

Will that need to be manually changed?

[tawdry vapor]

It's ignored by linting

[static canyon]

👌

[vale ibex]

@molten perch do we have easy docker migrations in api?

[static canyon]

Okay, so I think that's the only changes I need to do for site

Now I need to implement it in api

[molten perch]

Well, I believe the situation is the same there.
Except, alembic is a bit more lightweight, so you only need a database url and a token in the environment.

[vale ibex]

Alright cool, I might do the same I did for patsy in api then too, so contribs can just use the compose env

[molten perch]

Yeah, that would be really cool.

[vale ibex]

Should be easier in api due to alembic having all migrations in one path

I'll sort it after my trip to Nottingham

[molten perch]

Alright, also we should also discuss the async question, it's quite dubious in my opinion.

[vale ibex]

I might have missed the conversation about it

If you're around tomorrow I can give some opinions on it 😀

[molten perch]

Alright, cool. Have a good trip! 🙂

[vale ibex]

Thanks, fwiw I think async alchemy is fine, I wouldn't suggest sqlmodel for now

The alembic meta data seems to be lacking

[static canyon]

@molten perch ```py
C:\Users\tizzy\api>alembic revision --autogenerate - m "Add DM Sent field to infractions model"
'alembic' is not recognized as an internal or external command,
operable program or batch file.

[molten perch]

Yeah, haven't tested it out, might give it a go.

[static canyon]

Is this because of the poetry install thing again?

[vale ibex]

Try poetry run alembic...

[molten perch]

Before, running it.
docker-compose up postgres
Then, set two env variables (.env)

AUTH_TOKEN=tokeniguess
DATABASE_URL=postgresql://pysite:pysite@127.0.0.1:7777/pysite

(That's what I have)

You might not need the Auth_token, since it's decoupled from the api, but why not to add it if you can 😄

Sorry for the lack of docs, I might get around and write a few things, after the docker setup, and the dev-tools PR.

[static canyon]

pydantic.error_wrappers.ValidationError: 2 validation errors for Settings
database_url
  field required (type=value_error.missing)
auth_token
  field required (type=value_error.missing)

I do need both lol

[molten perch]

Oh, yeah sorry, there are a few version that I'm working with. 😄

Since you're not running the API,
Please do run:
alembic upgrade head
then
alembic revision --autogenerate - m "Add DM Sent field to infractions model"
We definitely have to change on that process 😂

[static canyon]

C:\Users\tizzy\api>alembic revision --autogenerate -m "Add DM Sent field to infractions model"
INFO  [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO  [alembic.runtime.migration] Will assume transactional DDL.
ERROR [alembic.util.messaging] Target database is not up to date.
FAILED: Target database is not up to date.

That's without the upgrade thing

alembic upgrade head gives an error

sqlalchemy.exc.ProgrammingError: (psycopg2.errors.DuplicateTable) relation "api_botsetting" already exists

[molten perch]

Yep, yep. Please run the upgrade, that's usually the part of the web container, and it runs automatically. But you only ran the postres.

Well, that seems impossible. Do you use the same database for the site and the API as well?

I tested it out on a freshly clone project, it seems to be working for me.

[static canyon]

Errr

[molten perch]

Maybe, you're still running the site DB? 😄

[static canyon]

DATABASE_URL=postgresql://pysite:pysite@127.0.0.1:7777/pysite
vs
DATABASE_URL=postgres://pysite:pysite@localhost:7777/pysite

So I guess

[molten perch]

Do you have docker-desktop? It might be easier to interfere with the containers for now.

[static canyon]

Yeah, I do

[molten perch]

Then, try to stop the site-db? 😄

[static canyon]

Done

[molten perch]

Okay, then it should be working now, I hope so.

[static canyon]

Ah wait I'm dumb

[static canyon]

I did this command in the site dir

That's why

[molten perch]

Might be problematic, yeah.

[static canyon]

Okay, starting

[molten perch]

🤞

[static canyon]

Seems to have worked

[molten perch]

What's the output? 😄

[static canyon]

Too long to send

[molten perch]

if you see migration messages such as: Initial migration then you're good to go, and can start run revision

[static canyon]

But the file was created 👍

[molten perch]

Okay, is there anything in it? 😄

[static canyon]

Generating C:\Users\tizzy\api\alembic\versions\3d9c7f4c3e4c_add_dm_sent_field_to_infractions_model.py ... done is the final line

Which has the def upgrade and def downgrade

[molten perch]

And, is there anything in def upgrade?

Since, alembic tends to generate empty migrations, if there's a problem.

[static canyon]

Yeah, there is stuff there

"""Add DM Sent field to infractions model

Revision ID: 3d9c7f4c3e4c
Revises: c551b4b47800
Create Date: 2021-11-10 22:26:00.101691

"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = '3d9c7f4c3e4c'
down_revision = 'c551b4b47800'
branch_labels = None
depends_on = None


def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.add_column('api_infraction', sa.Column('dm_sent', sa.Boolean(), nullable=True))
    # ### end Alembic commands ###


def downgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_column('api_infraction', 'dm_sent')
    # ### end Alembic commands ###
```to be exact

[molten perch]

Yaay 🎉 you might have to do some linting on it, since I was asked to add it to the linting workflow, but I might suggest it to be removed.
(Run “flake8 —config=tox.ini” (there is a pr that will make that easier, sorry))

[vale ibex]

Yea imo we should lint migrations

Just because they're autogenerated doesn't mean they don't need to be readable

[static canyon]

Yep, I'll do that```py
C:\Users\tizzy\api>flake8 --config=tox.ini
.\alembic\versions\3d9c7f4c3e4c_add_dm_sent_field_to_infractions_model.py:1:1: D213 Multi-line docstring summary should start at the second line
.\alembic\versions\3d9c7f4c3e4c_add_dm_sent_field_to_infractions_model.py:1:1: D415 First line should end with a period, question mark, or exclamation point
.\alembic\versions\3d9c7f4c3e4c_add_dm_sent_field_to_infractions_model.py:9:1: I100 Import statements are in the wrong order. 'import sqlalchemy' should be before 'from alembic import op'
.\alembic\versions\3d9c7f4c3e4c_add_dm_sent_field_to_infractions_model.py:19:1: D103 Missing docstring in public function
.\alembic\versions\3d9c7f4c3e4c_add_dm_sent_field_to_infractions_model.py:20:0: ANN201 Missing return type annotation for public function
.\alembic\versions\3d9c7f4c3e4c_add_dm_sent_field_to_infractions_model.py:25:1: D103 Missing docstring in public function
.\alembic\versions\3d9c7f4c3e4c_add_dm_sent_field_to_infractions_model.py:26:0: ANN201 Missing return type annotation for public function

[molten perch]

Alright, then I might have to modify the mako file as suggested before, to make it better.

[green oriole]

what about running black on them?

so they are somewhat pretty but we don't have to mess with autogenerated stuff?

[molten perch]

You can check out previous migrations to see a sample that passes.

[static canyon]

Yep

[green oriole]

hmm, I guess the docstring and annotations should be ignored, and we can add ISort for the imports

[static canyon]

That's why I have this open https://github.com/python-discord/api/pull/27/files

[molten perch]

Might be worth to open a PR I guess, but an issue would be good so that we can discuss

[static canyon]

Right okay, done

So do I now commit & push to the repo?

[molten perch]

Sure! I’ll review it tomorrow. (If someone won’t do it before me)

Go ahead! 🙂

[stable mountain]

You're the boss!

Your reminder will arrive on <t:1636584807:F>!

ID: 3455

[static canyon]

So this is all for bot#1864. How exactly does the "workflow" work in terms of now implementing this into the bot?

[dusky shore]

GitHub

Add DM Sent field to infraction details

[vale ibex]

No need, the imports and docstrings can be templated

Just fix it on the template and it's all good

[green oriole]

oh I see

[vale ibex]

It's jinja style templating

[molten perch]

You can, mention your corresponding PR’s, and something like “ported over from site, has to merged after the site pr” or something.

[molten perch]

You can’t modify the line lens. though.

[static canyon]

So I can PR the site & api changes without bot breaking?

[vale ibex]

Yea, that would be manual fixes, or black

[green oriole]

hey @vale ibex, have you made issues'n'stuff to murder @brisk belfry ?

[vale ibex]

Yes

On bot and k8s

[molten perch]

API is not yet used in production, as for api it’s fine, I’m not sure about site though.

[vale ibex]

I've made prs on both

You commented on one lol

[green oriole]

sick, that did ring a bell lmao

[static canyon]

Right, okay. I'll just add the "Do-Not-Merge" label for now

[molten perch]

Alright, nice work. I’ve gotta go though. Bye all! 😄

[static canyon]

Thanks for all your help 👍

[green oriole]

PR number?

okay we only have 9 site PRs

is there a site PR lol

ideally to port a new field to the site you'd want to add a sane default so we can have a nice cutover

[static canyon]

@green oriole since you're here, how do I use my edited database schema in bot?

As in to get support for the dm_sent field so that I can add it to the embed

[green oriole]

uhhhh

you do dirty stuff

[static canyon]

Not what I wanted to hear lmao

I was hoping it'd be easy

[brazen charm]

Wouldn't you just use it normally after spinning up the site with the change

[stable mountain]

It has arrived!

Here's your reminder: this
[Jump back to when you created the reminder](#dev-contrib message)

[static canyon]

So docker-compose up in the 2 repos?

[green oriole]

the easiest way is to use the same COMPOSE_PROJECT_NAME in both project env, start the site and then the bot

[static canyon]

Does it matter what value I use?

[brazen charm]

ah with separate containers

[green oriole]

no, just gotta be the same

mine is set to pydis iirc

[gritty wind]

You can keep them separate by setting network…

[static canyon]

And to start them is just this?

[gritty wind]

Or as Chris showed me

[green oriole]

you may need to specify bot and site manually

[gritty wind]

Modify the path in compose

[green oriole]

that's the easiest way I know to make them end up on the same vnet without modifying the config

[cold island]

start the site container as usual, but on the bot start only the bot

[green oriole]

maybe we could just set a pydis vnet in both compose file

[green oriole]

you don't have to, docker will see web is already up

[cold island]

Or run the bot on the host and make your life easier 👀

[green oriole]

lol not that bad of an option on linux tbh

[cold island]

Why specifically linux?

[static canyon]

oof

I got a whole load of errors

[green oriole]

we do have decent windows support now

[brazen charm]

never had any problems apart from bugs

[static canyon]

I keep getting this to start withpy metricity_1 | Traceback (most recent call last): metricity_1 | File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1276, in _execute_context metricity_1 | self.dialect.do_execute( metricity_1 | File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/default.py", line 608, in do_execute metricity_1 | cursor.execute(statement, parameters) metricity_1 | psycopg2.errors.DuplicateTable: relation "channels" already exists

[green oriole]

yeah, that's fair

[cold island]

https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/bot/#with-the-bot-running-locally Tizzy in case you're interested

[cold island]

Do you need metricity?

[green oriole]

so that's how that paragraph is named

[static canyon]

I... don't think so

I'm just trying to test this dm_sent field thing

[cold island]

Then disable it lol

[static canyon]

How?

[cold island]

i assume you have an env var to have it on

Since it's off by default

[static canyon]

Nope

Just BOT_TOKEN and COMPOSE_PROJECT_NAME

[cold island]

What command did you run?

[static canyon]

This is bot repo

docker-compose up inside of tizzy/bot

I guess just docker-compose run bot maybe?

🤔

[cold island]

huh... you do have this line right?
https://github.com/python-discord/bot/blob/main/docker-compose.yml#L50

[stable mountain]

docker-compose.yml line 50

USE_METRICITY: ${USE_METRICITY-false}```

[cold island]

No harm in trying

[static canyon]

Yepyml metricity: << : *logging restart: on-failure # USE_METRICITY=false will stop the container, so this ensures it only restarts on error depends_on: postgres: condition: service_healthy image: ghcr.io/python-discord/metricity:latest env_file: - .env environment: DATABASE_URI: postgres://pysite:pysite@postgres/metricity USE_METRICITY: ${USE_METRICITY-false} volumes: - .:/tmp/bot:ro

hmmm

Getting a load of other errors now

[static canyon]

PS C:\Users\tizzy\bot> docker-compose run bot
Starting pydis-testing_snekbox_1   ... done
Starting pydis-testing_redis_1     ... done
Starting pydis-testing_metricity_1 ... done
Creating pydis-testing_bot_run     ... done```It still runs metricity

When it crashes because of the error above it just restarts but by that point the site has crashed because of it

It keeps starting metricity when I don't want it to

Or I guess it's fine to run metricity just I get the error which makes it break

[cold island]

And it doesn't do that when you remove the compose project name?

[static canyon]

Lemme try

[static canyon]

If I remove COMPOSE_PROJECT_NAME then metricity doesn't start PS C:\Users\tizzy\bot> docker-compose run bot Starting bot_postgres_1 ... done Starting bot_redis_1 ... done Starting bot_snekbox_1 ... done

[cold island]

That's fascinating

[static canyon]

lol

[cold island]

@vale ibex feel free to ignore until later, but since you added the metricity env var do you have any idea?

[cold island]

Try setting USE_METRICITY to false explicitly

[static canyon]

What capitalisation is false?

Don't think it matters actually

Trying again

[cold island]

no caps

[static canyon]

PS C:\Users\tizzy\bot> docker-compose run bot
Creating pydis-testing_snekbox_1   ... done
Creating pydis-testing_redis_1     ... done
Creating pydis-testing_metricity_1 ... done
Creating pydis-testing_bot_run     ... done```

It's still running

BOT_TOKEN="TOKEN"
COMPOSE_PROJECT_NAME=pydis-testing
USE_METRICITY=false```

[cold island]

Sorry, I'm out of ideas for this hour of day lol

[static canyon]

lol

[cold island]

I suggest just running the bot on the host, makes things a lot simpler

[static canyon]

I'm also getting a 404 on site

web_1       | Not Found: /healthcheck
web_1       | "GET /healthcheck HTTP/1.1" 404 11455```

I'm going to bed now so I guess I'll look into this more tomorrow

[static canyon]

I'll try this tomorrow

!remind 10h https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/bot/#with-the-bot-running-locally

[stable mountain]

ROGER THAT

Your reminder will arrive on <t:1636623398:F>!

ID: 3456

[dim pelican]

Soooooo I think sir-lancebot#842 is finished. Should I dismiss Blue's review or wait for the re-review?

[dusky shore]

GitHub

Add color command

[rapid igloo]

nice! I think I'll test it again to make sure there aren't anything we missed

[dim pelican]

Please do, I've done a fair bit of testing but at this point I think having fresh eyes is needed lol

[summer garden]

Brad, What's the rationale behind having a special exception for hex but not the other modes

[dim pelican]

Regarding a special embed vs BadArgument?

[summer garden]

Yes, the other modes simply raise badarg

[dim pelican]

BadArgument was simpler honestly

Its a check if the values are in the correct range, and raise if something fails

[summer garden]

But how come it only exists for hex and not the others? Shouldn't it be consistent?

[dim pelican]

Ahh I see. I can change hex to badArg instead

[summer garden]

The custom embed makes sense

[dim pelican]

Well then its one change vs 5

[summer garden]

What I would suggest is a custom exception where we pass in the name of the mode and the possible values

I don't want to cause more work for you though

[dim pelican]

This is what I was about to test and commit:

        if len(hex_code) not in (4, 5, 7, 9) or any(_ not in string.hexdigits for _ in hex_code[1:]):
            # hex_error_embed = discord.Embed(
            #     title="The input hex code is not valid.",
            #     description=f"Cannot convert `{hex_code}` to a recognizable Hex format. "
            #     "Hex values must be hexadecimal and take the form *#RRGGBB* or *#RGB*.",
            #     colour=discord.Colour.dark_red()
            # )
            # await ctx.send(embed=hex_error_embed)
            raise commands.BadArgument(
                message=f"Cannot convert `{hex_code}` to a recognizable Hex format. "
                "Hex values must be hexadecimal and take the form *#RRGGBB* or *#RGB*."
            )

[summer garden]

Was there a reason you had an embed in the first place and not that? ^ I couldn't understand what you meant in your reply on the pr

[dim pelican]

I didn't have an error / BadArg raised for the hex command

Then I added the custom embed as discussed in the comments, but I think keeping the consistency and raising a BadArg makes more sense

[trail pilot]

I'll review it!

[summer garden]

We had badarg raised originally, no?

[dim pelican]

Depends on the commit you are looking at lol

[summer garden]

Hehe

[trail pilot]

Wait, I already did

[dim pelican]

This is the last commit before Hedy's review: https://github.com/python-discord/sir-lancebot/pull/842/commits/524d33a9909697386283d966dcda1dc712a16fb0

Ah crap

Wait, then why did I make a custom embed instead? Just changing the checks makes it raise the BadArg where before it did not

[trail pilot]

Line 144, if len(hex_code) not in (4, 5, 7, 9) or any(_ not in string.hexdigits for _ in hex_code[1:])::

Is there any reason why the identifier is _, since that's used as a throwaway usually from what I've seen, maybe use an identifier like letter or hex_letter

Sending this here because I wanna make sure this doesn't exceed the line character limit

[dim pelican]

One vs the Other:

The first one is consistent with the rest of the bad args

[dim pelican]

Its at 102 currently, could add that in

[trail pilot]

Awesome

[dim pelican]

digit

[trail pilot]

That's a better identifier actually lol

Regardless, LGTM
I already approved it from earlier though

[summer garden]

There was one line that lit up in my ide

I'm not at my computer right now

[dim pelican]

Oh boy

@summer garden which error message are you leaning towards? My opinion is the top one to keep it the same as the other commands

[summer garden]

Line 251 i think it was, where match could be undefined

[trail pilot]

My two cents: The top one is better to stay consistent

[summer garden]

I wasn't sure of this so I didn't comment. Can we move that line into the try block?

[dim pelican]

Ha

I was asked to move it out

[summer garden]

Hm

[dim pelican]

https://github.com/python-discord/sir-lancebot/pull/842/commits/1de5e9b905310eb4745b9b5c83561dea3a289ea9

[summer garden]

How come it should be outside?

[dim pelican]

https://github.com/python-discord/sir-lancebot/pull/842#discussion_r739592018

Took me a minute to find it

[summer garden]

I'm torn. I guess badarg is for arguement that are bad so making a custom exception doesn't make much sense

I don't know how this is usually done

[fallen patrol]

this

also yeah, the rapid fuzz matching thing if not implemented could probably be implemented in a seperate pr lol

[dim pelican]

Please lord lets keep it separate

[summer garden]

I could have a go at that some time

[dim pelican]

I got the rapidfuzz matching to work, and don't want to tackle another challenge /rewrite of it lol

[fallen patrol]

no you must rewrite this command 4 times before its merged /s

[dim pelican]

Haaaa I already have

[fallen patrol]

^6

[summer garden]

I'll try to decipher this tomorrow

I've tested the actual color conversions and those seem to be sound

Well done on this

[dim pelican]

Thanks! It turned out to be a monster and a great learning experience

[fallen patrol]

@dim pelican I left a review solely about documentation 😛

[fallen patrol]

yeah that's why I never did it 😓

[dim pelican]

Ahhh you reviewed before my last commit

They should all be "Create an embed..."

[fallen patrol]

#dev-log message

[dim pelican]

lol

If its cool with you I'll resolve all of those

[fallen patrol]

tldr those are what are shown to the user, so they should be useful for the end user

this one should not be resolved https://github.com/python-discord/sir-lancebot/pull/842/commits/aaad72a40b21b9d3b1d43d67a3206923722d8575#r747089330

other than that tho

[fallen patrol]

"User initiated command...."

the redundancy /blunt

I haven't seen a bot command that wasn't user initiated 😛

[dim pelican]

I'm going with the above @fallen patrol

[fallen patrol]

neat

...

how tf can i turn off github email notifications

[fallen patrol]

which page is that?

[dim pelican]

And un-check email in the notifications tab

Account Settings

[fallen patrol]

thanks

i should have done this sooner

[dim pelican]

RIP

[fallen patrol]

like

a lot sooner

[dim pelican]

I turned them off and created an email rule that sends them to a special folder

[fallen patrol]

this is a search for it

look at the scrollbar on the right

oh just like 1k emails from this one pr

[dim pelican]

I can believe it

[fallen patrol]

i don't check my email that much

[rapid igloo]

your client doesn't support threads? :P

[fallen patrol]

i think it does?

idk

and idc either

[fallen patrol]

outlook

[dim pelican]

Should be able to group conversations into single entries that can be expanded

[rapid igloo]

gmail does this, so it shows up as a single message

@fallen patrol https://github.com/python-discord/sir-lancebot/pull/842#pullrequestreview-803268423
I mentioned it my last review

https://github.com/python-discord/sir-lancebot/pull/842#issuecomment-965876151
lmao

[fallen patrol]

;-;

[trail pilot]

Yeah I get quite a few emails

Gotten 1,000 emails or so in the past 2 weeks

I think 500 of them have been from github

[vocal wolf]

@dim pelican Greetings, fellow human. Do you think it's possible for you to go through the reviews of the color PR to make sure that everything is resolved?

[dim pelican]

Salutations familiar life form, I believe everything in the reviews have been resolved, do you see one that is still outstanding?

BTW both Blue and Hedy have said they will test and review once more

[austere hornet]

Lol

[rapid igloo]

!remind 1H test brads color pr hopefully

[stable mountain]

Sure.

Your reminder will arrive on <t:1636599301:F>!

ID: 3457

[vocal wolf]

Very well.

[stable mountain]

It has arrived!

Here's your reminder: test brads color pr hopefully
[Jump back to when you created the reminder](#dev-contrib message)

[stable mountain]

It has arrived!

Here's your reminder: https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/bot/#with-the-bot-running-locally
[Jump back to when you created the reminder](#dev-contrib message)

[ashen isle]

@static canyon about the typehint tag. I wanted the tag to showcase that you can hint normal variables as well, not just function parameters. we could also try to keep the tag simple as a quick introduction and remove the temp variable entirely

[static canyon]

Yeah, I get why you wanted that type hint but it's not a good example

Variable type hints should only be used when the type isn't obvious

I tried to find an example on the bot repo but couldn't easily find one

[ashen isle]

yeah, I don't think we can fit a reasonable example in that tiny space. let's just put the return all on one line

[static canyon]

Perhaps another example following the lines ofpy data: list[str] = some_untyped_function() # we manually specify the type here since `some_untyped_function` doesn't have a return typehint so the program doesn't know what type it is

[ashen isle]

I'll have some time later today, I'll play around with some examples

[static canyon]

👍

[cold island]

I wouldn't say "only" here. Different code-bases have different guidelines

[ashen isle]

certainly not in the example we were using though

[stable mountain]

It has arrived!

Here's your reminder: g
[Jump back to when you created the reminder](#dev-contrib message)

[molten perch]

Hey, @hardy gorge (I'm sorry for the ping).
There have been discussions about the use of a test DB or mocked DB in the testing environment of the API (see: api#25)
What's your opinion on that?
(I'm looking to work on that PR this weekend if we manage to decide. 😄 )

[dusky shore]

GitHub

Migrate reminders endpoint from the site API, introduce model improvements

[gritty wind]

We ended up having a long discussion about it in dev-core. @last patio @obtuse arrow @green oriole can y'all put whatever you decided on in the issue

[hardy gorge]

That's a good question. I like being able to test with a production-like test database for integration tests, although I don't think it's necessary for all tests to be integration tests. You don't really get there if you only ever use a mock database, although you can get pretty close to the edge. (Some people call that edge-to-edge testing in the context of end-to-end testing.) A mocked database does allow you to test features independent of the database, which could be useful as well.

I don't think it's particularly hard to set up a test database. The ORM is capable of setting it up without running through all the migrations, although running through all the migrations also has its advantages if they define more than just the schema.

[molten perch]

No, it isn't. I had some free time and implemented the use of a test DB in the test environment, I was just waiting for the decision, which I'm still not sure about to be totally honest 😂

[hardy gorge]

There was a longer discussion in #dev-core, which should probably be leading here. I'm currently not really able to held up my end of the bargain in terms of being capable of taking some kind of lead role in this project, so it's probably be better to find another way of managing the project. But I'll have to discuss that with the admins as well.

[molten perch]

Yes, I understand. Then, I'll prepare my solution and push the new test suite.
Should be done by the end of tomorrow! 🙂

[molten perch]

One thing, before however..
So, for local testing there has to be a test database as well, and I did some research on creating an additional database in the docker-compose environment, but I ended up adding an additional test database container, I'm not sure about that solution though, that's the last thing I swear! 😄
(It's fine if the testing is done in the CI env. , I've set it up already.)

[gritty wind]

You can add multiple databases to the same docker container

[molten perch]

Well, how? 😄
With a SQL script, or similar?

[vale ibex]

Same psql container

just use CREATE DATABASE

So in the fixture, you could have the first step be the creation of a "test" database

Then you switch your database url to that newly created db and go as normal

Also, do you have to have your async stuff in a branch somewhere? 😄

[molten perch]

No, actually I tried to set it up locally but miserably failed to do it.
I'm not sure what the problem is, it wants to use pyscopg for some reason.

[vale ibex]

Do you happen to have the changes locally still?

I wouldn't mind giving it a go if you could push to a remote branch 😄

[molten perch]

I haven't done anything significant, just the alembic async config you sent me previously, 😦
But, I would be very grateful if you could somehow get it working.

[vale ibex]

I'll give it a look

[molten perch]

Also, sorry for my lack of knowledge in Docker, but where do I run this, again? 😄

[vale ibex]

not in docker

you connect to the db as you do with the db url

then create a new database using .execute("CREATE DATABASE test)

and then drop that connection and connect with the same url but with /test on the end

So instead of postgresql://pysite:pysite@postgres:5432/pysite you'd switch to postgresql://pysite:pysite@postgres:5432/test

[molten perch]

Oh, I see. I can integrate this into the pytest fixture I set it up, so that in the end it tears down the database.

Thank you! 🙂

[vale ibex]

👌

you can use urllib.urlsplit to split the connection url into it's parts

and then change the path

rather than messing on str.split

[molten perch]

Thank you, btw. another thing I had problem with during this async setup is pydantic and the “asyncpg+postgresql”

[vale ibex]

Is that due to the PostgresDsn thing we use?

I just came across that, so I made this ```py
class AsyncPostgresDsn(PostgresDsn):
"""Extension of Pydantic's PostgresDsn to only allow the "+asyncpg" schema."""

allowed_schemes = {"postgresql+asyncpg"}

just building the image now to see if it works

[molten perch]

Yes, I think. There was a pr that meant to solve this.

Oh looks fine. 😄

[vale ibex]

alright cool, the migrations run

lemme just try to make a migration

[molten perch]

Also, how would we run validations on the models if we use async sqlalchemy? ( see User model, role validations)

[vale ibex]

It lives poetry run task revision "Test migration." 😄

that ran fine

uhhh not sure, lemme look to see what they're doing

ah yea, that would need changing

[molten perch]

I don’t think that validations can be coroutines, so we might have to find a workaround

[vale ibex]

Yea from doing some searching it seems pydantic is against becoming async

so we would likely need to do any async validations in the routes

EG the role validator, we can do the negative one in the pydantic model, but the non-existent check would need to be done in the route itself

[molten perch]

Which route?

[vale ibex]

which doesn't seem overly unheard of

which ever route relies on the role given existing

[molten perch]

That validation is on the User model itself, it iterates through the array if role id’s and checks, if they’re existent.

Upon the creation of a user

[vale ibex]

Yea, so IG it's used by the POST/PUT method on some user endpoint?

so we'd defer that role check to within that route

since we're then in async land

[molten perch]

Alright then, I’ll make sure that it gets implemented when migrating this endpoint.

[vale ibex]

ah lol, that's why I was having issues finding where it was used

was just going to fix it myself now

[molten perch]

Yeah 😂 it’s still yet to be implemented, it’s gonna be the next endpoint 😄 although I’m not sure which endpoint it is.

@vale ibex Thank you for the PR, makes things way easier.
Because of the recent changes, and discussions I might have to change many things in the PR. So I might reopen it and start it from scratch. Would it be a problem?

[vale ibex]

The branch is on origin, so feel free to pull it and make changes to it directly 😄

Should make it a little easier on you too, rather than having to reimplement all of these changes

[molten perch]

Yes, I was gonna do this, since my current pr branch is messy already.
Before I start, one last thing.
Is there a preffered sqlalchemy syntax? 1.4 or 2.0?

[vale ibex]

could be better to go to 2.0 now, rather than having to migrate when more things are implemented

[molten perch]

Sure thing. Then I’ll definitely open a new PR. 😄

[vale ibex]

Put up site#622 for easy docker migrations (and pgdata volume cc @tawdry vapor )

[dusky shore]

GitHub

Easy docker migrations

[fallen patrol]

for snekbox, does the docker container have to be run in privileged mode?

[timid sentinel]

IIRC yes

Because nsjail needs it

For something

[fallen patrol]

yeah...

[molten perch]

Async SQLAlchemy is pretty good, I'm glad we decided to use it in the API

[vale ibex]

Yea I quite like it 😄

@router.post("/users/", response_model=UserRead)
async def create_user(*, session: AsyncSession = Depends(get_session), user: UserCreate):
    """Creates a new user."""
    db_user = UsersTable.from_orm(user)
    session.add(db_user)
    await session.commit()
    await session.refresh(db_user)
    return db_user

nice and clean code

Even more "complicated" stuff is nice ```py
@router.get("/users/", response_model=list[UserRead])
async def get_users(
*,
session: AsyncSession = Depends(get_session),
offset: int = 0,
limit: int = Query(default=100, lte=100),
):
"""Get a list of all users."""
result = await session.execute(select(UsersTable).offset(offset).limit(limit))
users = result.scalars().all()
return users

[molten perch]

Yeah, although I’m curious how the tests will work, if it involves setting up and tearing down databases. since, the nature of async.

[vale ibex]

This has a pretty nice guide for testing https://rogulski.it/blog/sqlalchemy-14-async-orm-with-fastapi/

using pytest

https://rogulski.it/blog/sqlalchemy-14-async-orm-with-fastapi/#tests

specifically

while it's specifically for sql alch 1.4, I don't think it'll be too different

[patent pivot]

think it's because we create cgroups yeah

[molten perch]

Ah I see, It looks good, although I’m not sure why was the session created twice and overriden in the fastapi app. We’ll see I might modify a few things

[patent pivot]

The biggest pain point is that snekbox needs to create parent cgroups for nsjail and therefore requires running the container in privileged mode. I've investigated ways to avoid this in the past, but was unsuccessful in finding any alternatives.
from mark

[vale ibex]

I think that was because of the way the injected the session into each route using fastapi depends

similar to how I've done it for patsy

[molten perch]

Oh fair enough, I just set the db url to be the test db url when creating the engine for the routes. Although an env variable had to be set, so that might be more convenient

[vale ibex]

Yea, the idea is so that you don't need to do ```py
with Session():
...

it just gets injected into the route automatically

[molten perch]

I see, first thing tomorrow! Thank you for all the help!

[vale ibex]

no worries 😄

[rapid igloo]

Is site#362 still up for grabs? I feel like it's a med priority as many websites start to have dark themes and reading guides (such as contributing tutorials) at night is really a pain unless they have stuff like dark reader.

This was the only thing I could find relevant about that issue #dev-log message so I thought I should ask about it before working on it 😄. I can give it a go for the CSS and bulma parts but would need some help for the toggle switch / oauth integration.

[dusky shore]

GitHub

Implement a dark theme

[vale ibex]

Doesn't seem like it, could you comment on the issue and I'll assign you 😄

[gritty wind]

Iirc, at some point someone was working on it, but never got back to it. Regardless, it's going to be a lot of CSS for whomever ends up taking this on

[rapid igloo]

shore done

[gritty wind]

As machoistic as it sounds, I now feel like writing some CSS. I should probably get forms back up and running now

[vocal wolf]

site#508 needs reviews.

does the bot not like me lol

https://github.com/python-discord/site/pull/508

^ needs reviews.

[gritty wind]

I think you crashed the bot /s

But no really, the bot is crashing

[vocal wolf]

heh?

[gritty wind]

Did we merge something

[vocal wolf]

on sir lance, yes

[gritty wind]

When, and what

[vocal wolf]

https://github.com/python-discord/sir-lancebot/pull/858

[gritty wind]

9 minutes ago

[vocal wolf]

this is the only thing that has been merged recently

[gritty wind]

And no start up message

[vocal wolf]

oh dear

[gritty wind]

I wonder if related

[vocal wolf]

ok I have an excuse

works on my machine

[gritty wind]

I don't actually know this is the cause

[vocal wolf]

should we reboot the bot?

[gritty wind]

Well, no. See #dev-oops it's crashing at startup

Lets see if I can get k8s logs from grafana

[vocal wolf]

ah

Let's try this again

site#508 needs reviews.

[dusky shore]

GitHub

offtopicnames active attribute and support for PUT and PATCH request

[vocal wolf]

there we go

[rapid igloo]

yeah I saw a lot of hard coded colors when I looked through the code base some days ago. I think I'll clean them up and assign variables or something. I'll also have to ask about some bulma and theme colors for dark mode, will ask when I get to them ~next week or 2

uhh @vocal wolf for the edit on github button I was already finsiehd working on it (I took a slightly different approach to the original PR) are you wanting to take over it? I'm happy for you to do it but I was using a better method and it only shows on articles, not both articles and categories

[vocal wolf]

tbh I have no idea how 2 site so you can take over

[short snow]

.bm

[rapid igloo]

how 2 site?

[vocal wolf]

I don't know frontend stuff and/or django

[rapid igloo]

haha alright it's ok

dawn already assigned me so I guess I'll PR my changes soon, might add some tests even though it's already 100% cov

[brisk brook]

Hol' up Sir Lancelot is just vibing in downtime hell?

[green oriole]

Is it still down

[short snow]

.help

[green oriole]

Well, it is up

I saw some messages earlier today about reverting something

I believe we pinned sir lance to à specific commit

[fallen patrol]

it was up a few minutes ago

.src

lol

i mean

someone restructured lance and i think that got merged lol

@green oriole https://github.com/python-discord/sir-lancebot/commit/2450138a5edb1e009e577d7da1134798261f26d3

https://github.com/python-discord/sir-lancebot/commit/2450138a5edb1e009e577d7da1134798261f26d3#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557L28-L30

removed the bot data mount??

[brisk brook]

Well, yes see Scale's PR

[green oriole]

It is currently pinned to 4224522, whatever that is

Or it is just kubenav being weird

[short snow]

Scale's PR doesn't fix the issue with DEBUG set to True in docker, does it?

[fallen patrol]

nope, just started

[fallen patrol]

run .uptime 😛

[green oriole]

It started the pinned version I believe

Let's see what deployed

[fallen patrol]

debug = os.environ.get("BOT_DEBUG", "true").lower() == "true"

shouldn't that default to 'false'?

[green oriole]

sir-lancebot@ec5eaadaae3e92f742f49085f782be40195a891c is the last deployment

Okay, that's indeed main

.uptime

[dusky shore]

I started up 3 minutes ago.

[green oriole]

Guess it do be solved

[fallen patrol]

lol

[green oriole]

Sentry is nice

I love their YouTube ad

[rapid igloo]

@dim pelican Re: https://github.com/python-discord/sir-lancebot/pull/842/commits/1914905bdc7bd591ff906562ee0496346d37a045#diff-141c9b3ff34e8fb41e45ebec131191459c020856b893e5a69ce69d2323aa148dR46 hey brad, did you test this? Wouldn't this strip off the last two digits of the input regardless of whether it was in the form of #RRBBGGAA or #RRBBGG?

[gritty wind]

No, somebody explicitly wanted it to default to true. We set it to false in prod

[gritty wind]

Nope, im going got delete file logging completely, just wanted to put out that fire first

[steady junco]

What will happen to the bots in this server after discord.py is gone

[patent pivot]

for now nothing

we'll see what options emerge down the line but there is no pressing need to change anything

[steady junco]

The module will be of no use

[patent pivot]

why not?

[steady junco]

You have to change the code

[patent pivot]

discord.py doesn't lose it's value because it's not maintained, everything that is there still works and works fine.

eventually we'll migrate, most likely we'll use a discord.py fork

[steady junco]

Oh so it won't be updated but we can still use it

[patent pivot]

so that we would already be somewhat compatible

[obtuse arrow]

There's no way to just turn the module off, it's just Python code wrapping the Discord API.

[patent pivot]

for now yes, I wouldn't encourage you to commit to it and I would explore the available forks like https://github.com/DisnakeDev/disnake

[steady junco]

Nice then

[static canyon]

I've made local changes to the site repo to add another field to the infractions database. How can I use this in conjunction with the bot repo to access & test the new field?

Context: https://github.com/python-discord/bot/issues/1864

[gritty wind]

How do you usually run site, and how do you usually run bot

[static canyon]

I've never run site before, and I do docker-compose run bot for bot

[gritty wind]

Ok, modify the site in the bot’s docker compose to point to your local site, instead of the remote container

You’re gonna want to copy the whole build part from bot basically, and modify it for site

[static canyon]

hmmm

[static canyon]

How do I do that?

[gritty wind]

Do you see the part in the compose that says image

Replace that with the “build” part from bot, then modify the context and docker file to point to where you have the site repo locally

[static canyon]

In the site repo?

[gritty wind]

This is all in the bot repo

[static canyon]

There's nothing that says image

ah wait

I see

  web:
    << : *logging
    << : *restart_policy
    image: ghcr.io/python-discord/site:latest```

I'm guessing you mean this

[vale ibex]

Yea, change image to site_web_1

that's assuming you have built the site image

[static canyon]

yeah i have

[vale ibex]

if not, cd into the site repo and do docker-compose build

👌

[static canyon]

right let's see

I run site first probably?

[vale ibex]

nope, if you have the site image built, just running the bot is fine

by changing that line you are telling docker to use the local image, rather than use the one in ghcr

[static canyon]

right, okay

It's doing stuff at the moment

Pulling web (site_web_1:)...

Just taking an age

[static canyon]

PS C:\Users\tizzy\bot> docker-compose run bot
Pulling web (site_web_1:)...
ERROR: pull access denied for site_web_1, repository does not exist or may require 'docker login': denied: requested access to the resource is denied```I think I do need to run because I'm getting this error?

It does exist

[vale ibex]

Could you show the docker-compose file?

[static canyon]

For bot?

[vale ibex]

- image: ghcr.io/python-discord/site:latest
+ image: site_web_1

that's the change you have right?

[static canyon]

  web:
    << : *logging
    << : *restart_policy
    image: site_web_1
    command: ["run", "--debug"]
    networks:
      default:
        aliases:
          - api.web
          - admin.web
          - staff.web
    ports:
      - "127.0.0.1:8000:8000"
    tty: true
    depends_on:
      - metricity
    environment:
      DATABASE_URL: postgres://pysite:pysite@postgres:5432/pysite
      METRICITY_DB_URL: postgres://pysite:pysite@postgres:5432/metricity
      SECRET_KEY: suitable-for-development-only
      STATIC_ROOT: /var/www/static```

yeah

[vale ibex]

ahhh, looks like the label is actually site_web

try removing _1?

[static canyon]

That worked

[vale ibex]

nice

[static canyon]

Thanks

So how do I now interact with this new database schema 😄

[vale ibex]

what did you add?

[static canyon]

dm_sent field to the infractions database

[vale ibex]

the _utils.py file has the actual call to the api

in post_infraction

and the get too

[static canyon]

merci

[vale ibex]

so that's where you'd add the param, but you'd need to go up the chain to where we try to send the dm to actually set it's value and pass it all the way through

[static canyon]

yeah

I'm relatively familiar with the infractions part of the codebase

[vale ibex]

Nice 😄

[static canyon]

Ah

We have a bit of an issue lol

apply_infraction is where we try to DM, but post_infraction gets called before apply_infraction

So I guess I just need to change it so that we apply first

I don't particularly understand why we post being apply anyway

Hmm

I don't understand this lol

@vale ibex please help lol

I don't know how to get dm_sent into post_infraction

Unless we POST then PATCH or something put that's probably not a good thing to do

[green oriole]

I would say patching makes sense here actually

[static canyon]

Time to double the amount of API requests then

I'll have to figure out how to do that

[green oriole]

Send a PATCH at bot/infractions/:id with the json body {"dm_sent": true}

[static canyon]

Merci

!remind 14M ^

[stable mountain]

Sure thing!

Your reminder will arrive on <t:1636718102:F>!

ID: 3461

[static canyon]

!remind edit duration 3461 40M

[stable mountain]

Sure thing!

That reminder has been edited successfully!

ID: 3461

[static canyon]

Thinking about it what I'll do is default to making it false and then if the dm gets sent do the PATCH. Which I realise is probably what you originally meant and I probably just misinterpreted

[green oriole]

I didn't think about that but yeah that's smart

[stable mountain]

It has arrived!

Here's your reminder: ^
[Jump back to when you created the reminder](#dev-contrib message)

[static canyon]

Is the : needed? i.e.py await bot.api_client.patch( f"bot/infractions/:{infr_id}", json={"dm_sent": True} )

[gritty wind]

Nah

Just the ID

[green oriole]

Nah nah, it was to specify you need to enter your ID variable here, not a literal "id"

[static canyon]

👍

So f"bot/infractions/{infr_id}"

async def notify_infraction(...):  # pre-existing function
    ...
    dm_sent = await send_private_embed(user, embed)  # line was previous just `return await ...`
    if dm_sent:
        await bot.api_client.patch(
            f"bot/infractions/{infr_id}",
            json={"dm_sent": True}
        )
        log.debug("Updated infraction #{infr_id}. `dm_sent` field set to true.")

    return dm_sent```this is the approach I've gone for

[short snow]

which isssue is this? So I can get some context on the discussion

[static canyon]

bot#1864

[dusky shore]

GitHub

Add DM Sent field to infraction details

[short snow]

👍

[static canyon]

I'm thinking this should work

I just need to add processing for the GET now

[short snow]

yeah that would work, and btw you are missing the f on the logging statement 🙃

[static canyon]

Yep, thanks 👍

[short snow]

Also for the api PR, it should probably default to False

as we patch it later as True if it is sent, originally we don't set it

Also could you run black, there is no workflow for it yet.. planning to make a PR soon if no one gets to it before

[static canyon]

originally we don't set it
I do actually

[short snow]

Ah ok but still I don't see the need to set it originally

[static canyon]

I just added dm_sent: bool = False, to post_infraction and then "dm_sent": dm_sent to the payload

[short snow]

ah ok 👍

[green oriole]

@gritty wind biased poll lmao

[gritty wind]

It's been fixed smhhhh

[short snow]

lol

[gritty wind]

It's not strictly a democratic poll btw. I've never used it, but I'm open to listening if someone thinks it's important

[static canyon]

I think I might be done with this now

Just need to test it all

[green oriole]

I guess it is more a matter of who use it and why, rather than who doesn't

[gritty wind]

I feel like we're going to see that most don't. We can try to accommodate those who do though. But if this poll goes as expected, at the very least file logging will not be enabled by default and will get it's own flag

[green oriole]

that's probably a smarter solution, yeah

[gritty wind]

It's a valid solution, but if no one uses it, I don't think we'll bother tbh

We'll see

[short snow]

I only used it once, when I cleared my terminal by mistake and wanted to see my error traceback and my debug logs

[gritty wind]

Yeah I figure something like that is most likely to be the reason people might use it, but it's a bit niche

[clever wraith]

Ho guyz

[obsidian patio]

@gritty wind would it be possible to make it user-togglable? Is there any way that would reduce the maintenance burden?

[short snow]

we can just add a env variable ro enable/disable it, it won't really be a maintenance burden

[patent pivot]

we already have that

and it still knocked out prod earlier today lol

[brazen charm]

I do find it useful to be able to look into older logs

[obsidian patio]

Gotcha

[brazen charm]

although I could also just redirect the output from pycharm

[patent pivot]

fwiw, we don't do it in prod because our prod filesystem is read-only

so that's why we have the toggle there already

[short snow]

ah I missed null=True, we don't need that if you always pass on dm_sent

[gritty wind]

We'll still want to give older infractions a sensible default

[short snow]

hmm so null sounds the best, 👍

[static canyon]

Yeah, the null=True was for for prior-infractions

We can't default to False there since that could be incorrect information (we don't know if it sent or not)

[short snow]

yeah

[green oriole]

@gritty wind FYI I do have a reasoning against it, when running inside Docker, since the container is running as fakeroot the bot file it creates will be owned by root, which could be a big problems for users that don't have root access on their machine, as they won't ever be able to delete the file

even with root access it is a pain

[static canyon]

How can I make the bot repo use my custom schema for site?

I've updated site but I've tried a number of different things in bot but it won't run my site

[short snow]

are you running bot with docker?

if yes, it would be using the ghcr site image

[static canyon]

I've changed that in the docker-compose but it's still not using the right site

Because when I run site on it's own and then send a GET to /api/bot/infractions it returns an empty list but when I do it in the bot it returns stuff

So it's using a different site

[short snow]

whats your docker-compose?

You should run site separately in docker, and run bot with poetry since the only needfor docker running is redis and site, you don't need rdis now so you can skip that

[static canyon]

  web:
    << : *logging
    << : *restart_policy
    image: site_web
    command: ["run", "--debug"]
    networks:
      default:
        aliases:
          - api.web
          - admin.web
          - staff.web
    ports:
      - "127.0.0.1:8000:8000"
    tty: true
    depends_on:
      - metricity
    environment:
      DATABASE_URL: postgres://pysite:pysite@postgres:5432/pysite
      METRICITY_DB_URL: postgres://pysite:pysite@postgres:5432/metricity
      SECRET_KEY: suitable-for-development-only
      STATIC_ROOT: /var/www/static```this is the relevant bit

[short snow]

share the Db section also, I am not completely sure btw

[static canyon]

Left that defaultyml services: postgres: << : *logging << : *restart_policy image: postgres:13-alpine environment: POSTGRES_DB: pysite POSTGRES_PASSWORD: pysite POSTGRES_USER: pysite healthcheck: test: ["CMD-SHELL", "pg_isready -U pysite"] interval: 2s timeout: 1s retries: 5

[gritty wind]

Could you share the logs from docker compose up -d web

[short snow]

Ah, use the DB of the site compose, site_postgres IIRC

[gritty wind]

It shouldn't really matter, the site will migrate any DB to work

[short snow]

since that has the schema changes

[gritty wind]

Site migrates DB at startup

[static canyon]

Right, yeah

image: site_postgres #postgres:13-alpine

I'll try this

[gritty wind]

It could work, just by virtue of the data being in one db, but not the other

[static canyon]

PS C:\Users\tizzy\bot> docker-compose run bot
Pulling postgres (site_postgres:)...```seems to be working

[short snow]

The DB migration happens during the image formation ig

[static canyon]

Ah, the name is wrong PS C:\Users\tizzy\bot> docker-compose run bot Pulling postgres (site_postgres:)... ERROR: pull access denied for site_postgres, repository does not exist or may require 'docker login': denied: requested access to the resource is denied

Lemme see what it is

It is the postgres:13-alpine lol

[short snow]

just run the bot with poetry lol, and update the URls to be the correpending ones from site compose

[gritty wind]

It's trying to pull from a remote, check the name

[static canyon]

What it was is my version

So that's what I had it as and it wasn't working

[gritty wind]

that'll do nothing, it'll just try to use the pg:13 image from docker hub

The problem isn't with your DB

[static canyon]

From which repo?

[gritty wind]

Bot

[static canyon]

Okay

[static canyon]

PS C:\Users\tizzy\bot> docker-compose up -d web
Starting bot_postgres_1 ... done
Starting bot_metricity_1 ... done
Starting bot_web_1       ... done```

[gritty wind]

nope

[short snow]

ah ok

[gritty wind]

Could you get the actual logs? It should be easiest to do from the docker desktop UI

[static canyon]

I think this is what you want https://paste.pythondiscord.com/hepemosuma.properties?noredirect

[short snow]

you could probably get in your bot_web shell and see if it contains your changes, that could be the only issue if DB isn't it

[static canyon]

L417 is the migration to my database

So it should be using it(?)

[gritty wind]

So I'm seeing that it didn't find your migration

[static canyon]

It did

web_1 | Applying api.0075_infraction_dm_sent... OK

[gritty wind]

Oh nevermind

[static canyon]

L417

[gritty wind]

What is the problem exactly

[static canyon]

I don't know just the infractions don't have the field

[gritty wind]

Did you modify the viewset

[static canyon]

Yes

probably not actualy

[gritty wind]

Are your changes pushed somewhere

[static canyon]

I added it to the Serializer

Lemme push real quick

[static canyon]

site#621

[dusky shore]

GitHub

Add dm_sent field to infractions model

[static canyon]

Why is flake8 line limit 100 instead of 120 on site lol

[green oriole]

Nobody knows lol

[gritty wind]

Your code is fine

Do you actually have an infraction in the DB

(The one in the bot compose)

[static canyon]

I should do

Lemme start up the bot

PS C:\Users\tizzy\bot> docker-compose run bot
Starting bot_snekbox_1  ... done
Starting bot_postgres_1 ... done
Starting bot_redis_1    ... done
Starting bot_metricity_1 ... done
Starting bot_web_1       ... done
Creating bot_bot_run     ... done```

PS C:\Users\tizzy\bot> docker-compose run bot
Starting bot_postgres_1 ... done
Starting bot_snekbox_1  ... done
Starting bot_redis_1    ... done
Starting bot_metricity_1 ... done
Starting bot_web_1       ...
Starting bot_web_1       ... error

ERROR: for bot_web_1  Cannot start service web: driver failed programming external connectivity on endpoint bot_web_1 (8a9e308e78ee023ae1be1b26d594be0a0e0a662d7e99225e13d651877818587e): Bind for 127.0.0.1:8000 failed: port is already allocated

ERROR: for web  Cannot start service web: driver failed programming external connectivity on endpoint bot_web_1 (8a9e308e78ee023ae1be1b26d594be0a0e0a662d7e99225e13d651877818587e): Bind for 127.0.0.1:8000 failed: port is already allocated
ERROR: Encountered errors while bringing up the project.```getting this now

[gritty wind]

Do you still have them running in another container

[static canyon]

The issue is that it starts it twice

[gritty wind]

What makes you think that?

[static canyon]

It has the thing twice

[gritty wind]

That's just how docker formats the output

It's only trying to start it once

check the output of docker container ps

[static canyon]

ah

PS C:\Users\tizzy\bot> docker ps
CONTAINER ID   IMAGE     COMMAND   CREATED   STATUS    PORTS     NAMES

Now got this

https://paste.pythondiscord.com/quvicitodo.apache?noredirect

@gritty wind

It's complaining that site isn't running lol

And if I make it run then it complains that it is running

What do I do?

[gritty wind]

What did you run when you got that?

[static canyon]

docker-compose run bot

[gritty wind]

well, just docker-compose up -d

or up

yeah up

start? I'm blanking on the command

Yeah run is something different, we want up

[static canyon]

Nope

Same error

I think I know why

[gritty wind]

Do you have something somewhere running on that port? W/e, we can brute force our way through by setting a different one

[static canyon]

This was all working

I'm so confused why it's not now

[gritty wind]

You probably have it running in the background somewhere within your system. A reboot would in theory sort it out. If it's still in docker, you could check docker desktop's UI to look for anything that might be running.

Otherwise, for now just change the port

[static canyon]

There's nothing running in docker

I guess I'll restart my PC

Be right back

Let's see

nope

Not happening

Ffs

[gritty wind]

Just change the port

[static canyon]

How?

And in which repo?

[gritty wind]

Line 76, in bot. Almost everything is in bot, always

- "127.0.0.1:8000:8000" -> - "127.0.0.1:8001:8000"

[static canyon]

Which file?

[gritty wind]

Docker compose

[static canyon]

Okay, done

Now docker-compose up?

[gritty wind]

Yeah give it a shot