#Development Chat

1 messages · Page 5 of 1

wild marlin
#

Will merge that one and the other small pr into next when they are on main

silent marsh
#

uh

#

could we please merge next into main?

#

it's not like we'll do another patch release soon anyways, i'd rather release 0.17 immediately

karmic aurora
#

I'd like to maintain them as separate until the release

#

it doesn't cost us anything, we can just cherry-pick that next commit to main for example

primal torrent
#

honestly with the way twilight dev is going for a while now: next should probably be the default branch

#

and main could be like a "stable" branch instead for backports

karmic aurora
#

I'd like to increase our release interval (with the new workspace publish feature) up to one stable release per pr

#

next is currently in a bit of a special state with the major changes being to the rate limiter, but with almost no one directly using that and experiencing breaking changes

silent marsh
#

my PR that switches to edition 2024 will break everything

#

it needs complete reformatting of all files

#

merging that over is gonna be pain

#

and the way i understood it, we wanted to do a new release this weekend

#

or at least that's how i interpreted Erk

wild marlin
#

Yeah i am in the garden atm, but a release this weekend is my goal

#

And yeah maybe the pr's I made should just be retageted next and merged in and then next to main?

primal torrent
#

i think in general everyone targetting next and then cherry picking or merging next into main as needed is an easier workflow

silent marsh
#

yeah

#

let's just make sure next is clean on top of main

#

and do all the changes there, then merge into main for the release

#

that would make my life so much easier

primal torrent
#

like rename next to "develop" and main to "stable" for example

silent marsh
#

hm

#

@wild marlin you were planning to relase 0.17 right?

#

i'll just drop the hacks for crypto providers in twilight then

wild marlin
#

Yeah

#

I don't think we should do another 0.16

silent marsh
#

yeah i think so too

wild marlin
#

Should I do the merge to main now? or wait until a release?

karmic aurora
#

I think we merge w/ the changelog

silent marsh
#

i guess tests will still fail, i'll fix that once i know whether they still do

#

wait i think i messed up lol

#

Tests should pass now as well

#

I need to go, but should be ready now

#

AAAA simd-json 0.16 went to an MSRV of 1.88

ivory portal
#

Is anyone already working on implementing label components? I started working to contribute to that today but I realise I should probably ask first if someone is already taking it on

wild marlin
#

I don't think anyone is

#

That was a quite boring pr for 98% of the files :P

image.png
#

I think we should document how we want people to use TLS somewhere

silent marsh
#

i have one more change to amend for the PR

#

i'll force people to use a process default crypto provider in twilight-http as well while i'm at it

#

pushed that on top though so you can easily review it

wild marlin
#

I guess discord does not do anything with client certificates?

#

Yeah I am not even sure how that would work for bots? You download a certificate from the developer page or something

karmic aurora
#

I found at least one nit in #2450, going through the rest of the files right now...

silent marsh
#

I'll fix the CI once I'm back, gotta go vote in local elections quickly

primal torrent
wild marlin
primal torrent
#

i'm taking a quick look at prs that still need aproving, guess i'll start with that one

#

there approved, its just adding a path

#

also we should probably do a purge of old prs at some point

karmic aurora
silent marsh
#

I'll take a look at #2453 once I'm back

primal torrent
#

public api breaking or just internal api breaking?

karmic aurora
#

As for testing, I stand firmly behind that my PRs are at least less broken than what's in next atm

primal torrent
#

also nothing prevents a 0.18 in like 2 weeks or so

wild marlin
#

tbh I would personally rather just wait till tomorrow or tuesday with a release

karmic aurora
#

our Path enum is fundamentally broken in that it doessn't account for the http method, and likely silently broken in other ways

wild marlin
#

I am not steadfast on it being today as we still need a couple of things such as some release notes

karmic aurora
#

I'm working on bringing http-proxy up to date with #2453 FYI

karmic aurora
#

opening the files list of #2450 pegs that chrome tab's thread to 100% for nearly a minute

silent marsh
#

lmao

#

once CI passes I'll rebase and merge

wild marlin
#

If I use the "New Experience" that also happens in firefox lol

silent marsh
#

oh i messed up the commit message

#

w/e

#

for some reason it used the PR title instead of the ONE commit's message that is part of the PR

karmic aurora
#

rebased my prs

silent marsh
#

i think #2453 is ready to go

#

really nice PR

#

#2455 should also go in but has conflicts after #2453

karmic aurora
#

resolved

silent marsh
#

okay, awesome

#

once that one is in i think 0.17 can be released

#

i'll look at #2322 now

karmic aurora
#

it's marked as a draft until 0.17 goes stable

jaunty jewel
#

I will take some time to look at #2461 tonight

#

as for #2438 I’ll try to find time and complete that

ivory portal
ivory portal
#

From what I understand, #2450 made changes such that it's now necessary to call CryptoProvider::install_default manually. I think the documentation does not reflect that yet with the documentation on features still saying with the respective features you don't need to manually install a crypto provider even though the features don't exist anymore. Also the examples on next currently fail because no crypto provider is installed.

karmic aurora
karmic aurora
wild marlin
wild marlin
#

Ahh I guess you got it fixed in tokio-websockets

karmic aurora
#

Yes, we just need to update our docs about rustls' crypto providers (though rustls will panic w/ an explanation too if misconfigured)

silent marsh
#

okay, tws 0.13 is tagged

wild marlin
#

Nice

silent marsh
#

sadly this means another MSRV bump in twilight but this time from 1.88 to 1.89

#

not that i think anyone would really care, but the diff for switching to edition 2024 was way too large

wild marlin
#

Tbh I think it would have been a okay doff if formatting was in its own commit

wild marlin
#

@karmic aurora Did you add the new rustls documentation somewhere?

#

With how you need to install the provider

karmic aurora
#

As long as you depend on rustls with default flags it will be auto-installed

karmic aurora
wild marlin
#

👍 We should maybe highlight it somehow since it is a quite uncommon pattern

karmic aurora
#

It should definitely be highlighted in the changelog

karmic aurora
#

For #2464, I tried splitting the PR such that only the first commit modifies the zlib/zstd implementations

silent marsh
#

re: zlib stuff, we should remove zlib-stock and use zlib-rs in flate2

#

zlib-ng is also now really only a valid choice on s390x

karmic aurora
silent marsh
silent marsh
#

what's missing for 0.17 now?

#

i think we should be ready

jaunty jewel
wild marlin
#

I am on holiday to 🇰🇷 for the next 2-ish weeks so I am probably not going to have much time for Twilight

lean geyser
#

very exciting, hope you try some good food.

karmic aurora
silent marsh
#

I'm currently on vacation until Sunday and I think we shouldn't delay this any longer

wild marlin
ivory portal
#

I'll work on file upload modal components this weekend, if new modal components isn't getting reviewed/merged before then I'll just add it to that mr

jaunty jewel
karmic aurora
#

What's even the impact of boxing the top-level event variants? I feel like people still do

match event {
    Event::MessageCreate(message) => on_message(message.0).await?, // The inner field is _not_ boxed!!!
    _ => unimplemented!(),
}
#

I have to import less items from twilight by using Message over MessageCreate, Channel over ChannelCreate, etc and I don't have to include Box<> in my event handlers.

#

Oh and I'm not proposing holding up 0.17 for this

jaunty jewel
#

let's try getting #2461 and #2467 merged asap

jaunty jewel
wild marlin
#

Going to be back home on thursday or friday and then I can start to have a look at some of the things

jaunty jewel
wild marlin
#

Development Chat

wild marlin
#

@south forum Is there a reason that you did not update the role_colors struct?

south forum
south forum
south forum
#

CONTRIBUTING.md says you should add labels yourselves but contributors don't have access to do so, is there any command or such to add labels? (e.g d-api)

wild marlin
#

We don't have that, just added them to your pr, but we should think about how we could do that properly.

wild marlin
#

@ivory portal Do you think you will have the validation changes done sometimes today?

ivory portal
#

yeap, working on it right now

wild marlin
wild marlin
#

I will start the release process now I think, so I will merge next into main and then begin to set everything up and get some change logs written

south forum
wild marlin
south forum
jaunty jewel
wild marlin
#

@karmic aurora u8 or u16? the maximum value is 50 so I think I will just make it a u8

karmic aurora
#

There's precedent for u16

#

I guese for future compat

wild marlin
#

Just going to merge it in a bit

south forum
wild marlin
#

I did actually write a review comment about it, and then deleted it ferris_sweat

#

@south forum Is there a reason you did not add the fields to the partial member struct?

south forum
south forum
wild marlin
#

I wonder why we use partial member there

south forum
#

just two structs to update in this case, which I don't personally see any reason for

wild marlin
#

** member is sent when the interaction is invoked in a guild, and user is sent when invoked in a DM

#

ah nvm

#

Yeah I am not sure either

#

just to have the permission included in only one of them?

south forum
south forum
wild marlin
#

interactionmember should probably be removed

south forum
wild marlin
# south forum y

because the user is not sent in the member field as I read it as first

south forum
#

I'll just remove it in this pr ferristhumbsup

#

since it's about members anyways

primal torrent
#

Iirc they used to have less info

#

But discord expanded them later

#

Interaction does have some extra permission fields, unless they are elsewhere

south forum
primal torrent
#

Yeah thats one downside of rust

#

Docs do need to make it very clear then when itll be there

south forum
south forum
#

Actually the InteractionMember begins to make sense now because this is different per endpoint

wild marlin
#

Ahh maybe that is the reason

#

Tbh when you have not looked at a piece of code for a few years you kinda forget some details vikingblobsved

south forum
wild marlin
karmic aurora
#

@wild marlin re #2470:
I want to also add a Bucket::is_shared method, but I feel unable to properly explain the method. I'd like to link to a guide-level explanation of shared limits, but there's none at the moment. I plan to do a large documentation overhaul, so would you like to defer adding the method until then?

/// Whether the bucket is exhausted due to a [shared] limit.
///
/// [shared]: RateLimitHeaders::shared
pub const fn is_shared(&self) -> bool {
    self.limit == 0
}
primal torrent
#

That method seems wrong

#

Shared is determined by a separate header

wild marlin
karmic aurora
#

It works out as the rate limiter just waits for the limit to elapse

#

It could be expressed as an enum, but I'd like to not to input validation

enum Limit {
    User(NonZero<u16>),
    Shared,
}
south forum
#

how's the changelog for 0.17 going? :)

wild marlin
#

Going to do 0.17 now probably

eternal owl
#

hell yeah

south forum
wild marlin
#

Twilight Version 0.17

New release of the Discord library system Twilight.

Highlighted Changes

Support for components V2. (#2422)

Long awaited support for the new components in Discord.

Support for new modal components. (#2461)

Support for new modal components including file upload.

New Ratelimiter (#2418), #2453))

The http ratelimiter was rewritten to be better and simpler.

It was also made to support unknown paths so it can now be used with arbitary paths and not only some already known.

RusTLS Changes

Across multiple crates we have removed the features that enable a
specific cryptography backend and we will now instead leave the
responsibility to the user to enable one.

For example if you want to use ring you can add the rustls crate
with the ring feature and add the following to initialization of the
cryptography provider:

rustls::crypto::ring::default_provider().install_default();

Or similarly if you want to use aws_lc_rs you can add:

rustls::crypto::aws_lc_rs::default_provider().install_default();

Other possible crypto providers is for example Microsoft's Symcrypt.

Gateway compression changes

For compression the new zstd feature have been added to use the new
more efficient ZStandard compression algorithm.

The features zlib-simd and zlib-stock was removed in favour of zlib.

#

^Overall changelog

#

Ah we may have to make this change:

note: this call may capture more lifetimes than intended, because Rust 2024 has adjusted the `impl Trait` lifetime capture rules
   --> bot/src/main.rs:84:18
    |
 84 |     let shards = create_recommended(&http, config, config_callback).await?;
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: if you can modify this crate, add a precise capturing bound to avoid overcapturing: `+ use<F, Q>`
   --> /home/erk/.cargo/git/checkouts/twilight-44fa90962851ed77/9f305ca/twilight-gateway/src/lib.rs:176:13
    |
176 | ) -> Result<impl ExactSizeIterator<Item = Shard<Q>>, StartRecommendedError>
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: clone the value to increment its reference count
    |
 84 |     let shards = create_recommended(&http.clone(), config, config_callback).await?;
    |                                          ++++++++
silent marsh
#

MSRV + edition change is missing

wild marlin
#

If someone wants to review it, otherwise I just merge it when I am done with updating the changelog with the above stuff

south forum
#

will there be a CFT?

wild marlin
#

Well not really, I don't think I want to make a beta release.

south forum
wild marlin
karmic aurora
# wild marlin Going to do 0.17 now probably

RusTLS Changes
Should be pointed out that a default provider can be implicitly installed through rustls' feature flags
Gateway compression changes
Should be pointed out that zlib-simd is now always used

wild marlin
karmic aurora
#

yes

#

and it defaults to aws_lc_rs

wild marlin
karmic aurora
#

so you should just have to depend

wild marlin
#

It install them at first use right? so you can still do it manually right?

karmic aurora
#

yes

#

exactly

wild marlin
#

okay going to start now

#

🤞

karmic aurora
#

lmk if you need my input on anything

wild marlin
#

Cargo-release already failed blobsved

karmic aurora
#

crap

wild marlin
#

note to self, stop using cargo release and make some scripts for cargo publish --workspace

#

I think everything is released now

wild marlin
south forum
wild marlin
#

Ahh yeah that would be a good idea

#

The book actually needs a few updates here and there

#

I don't know if I will get to those today, but I might aim for tomorrow.

south forum
wild marlin
karmic aurora
#

yes 👍

karmic aurora
wild marlin
karmic aurora
#

oh right we do one review for the proxy

#

I'll do it

#

Ah I'd have liked to not squash those commits

#

we'll they're still there in the PR body

sage cloud
#

@wild marlin you want that cargo update as a separate commit or squashed in gateway-queue#32?

wild marlin
#

Ah looks like the dockerfile fails

sage cloud
#

I didn't check that after cargo update oops

wild marlin
#

@silent marsh We could just use the one from http-proxy with minor changes right?

sage cloud
# 
Failed to connect to musl.cc port 443 after 76543 ms: Could not connect to server

what the hell

wild marlin
sage cloud
wild marlin
sage cloud
#

Ahh gotcha

#

Can easily adapt that Dockerfile by the looks of it yeh

wild marlin
#

Fine to do in the same pr if you want otherwise I can get around to doing it tomorrow

sage cloud
#

It's already pushed 🫡 @wild marlin

wild marlin
#

no I mean .github/workflows/docker.yml

sage cloud
#

Yeah, seems that the arch has changed from armv8 to arm64/v8

#

will have to fiddle with how that needs to work

wild marlin
#

I think its probably easier to pretty much copy the one from http-proxy and change the paths/names to be gateway-queue

#

Okay now it looks better

sage cloud
#

Yep looks to be building successfully now

silent marsh
wild marlin
#

Okay merged it in so now we see if everything works of if I need to configure some secrets

primal torrent
#

has anyone ever looked at why twilight takes so long to compile?

#

in dev:

  • twilight-gateway: 2.2s
  • twilight-http: 1.9s
  • twilight-model: 6.2s
    in release:
  • twilight-gateway 30.7s
  • twilight-http: 10.8s
  • twilight-model: 7.8s
#

in release it speds a ton of time in codegen

wild marlin
#

We don't really do anything fancy in the gateway

#

maybe it is the async functions that makes it blow up?

#

Is that numbers from --timing

primal torrent
#

yeah

#

like these are some crazy numbers

image.png
#

especially since total build time was 62s. the gateway allone was nearly half

wild marlin
#

Do you set anything in your .cargo?

primal torrent
#

copy of settings to speed things up like using mold and 8 threads

wild marlin
#

So not codegen-units?

primal torrent
#

nope

#

looks like not using simd-json shaves about 2s off the gateway

#

numbers are about the same with nothing in my .cargo

#

makes sense cause my .cargo changes are more ment to help incremental builds and running doing more in parallel, not speeding up individual crates

karmic aurora
#

I'm able to reproduce this w/ gateway-request-members

#

I tried replacing next_event w/ next in gateway-queue-http (such that the gateway does not require codegen for Event::deserialize, which is massive) and I still got the same result

karmic aurora
#

I don't know how to continue troubleshooting...

wild marlin
primal torrent
# wild marlin <@131517556786855937> If you have a setup could you try and see if https://githu...

it's a very basic early rework project, the datastore and main being that high up are weird but the don't take nearly as much time in the total compilation times. but that the third one being twilight http is odd. especially since there are only 2 http calls to the code in this little sandbox project: one to get the application info, and one to get the authed gateway info for shard recommendations

📎 llvm-lines.txt
#

i wonder if more gateway shows up if i spin up a shard thinkGate

wild marlin
#

I did some digging down into llvm and similar some time about because I had that project that went from a few seconds to 30 minutes when I used --release, will probably see if I can find the commands I used back then and see if anything sticks out.

karmic aurora
#

I ran llvm-lines and cargo-bloat, but they don't track compilation time <-> code size

#

I.e. the gateway does not produce that much code, but it receives disproportionately more time being optimized

primal torrent
#

Wonder if its generics shenanigans

silent marsh
#

I don't think the gateway abuses generics that badly

#

The cache definitely does abuse them a fair bit

primal torrent
#

there are a bunch of prs open from early 2024 and older, might it not be time to just close them?

#

some seem useful in theory but all are prett much abandoned drafts full of conflics you'd have to pretty much start over for anyways

jaunty jewel
#

I will find some time to start over for #2438 say after mid-December

#

as for #2314 I probably will not work on it now

jaunty jewel
jaunty jewel
jaunty jewel
#

How will we proceed with #2004?

karmic aurora
#

It may be combined with a decoupling of the client and request builders #general message

#

I do not like the InteractionClient API

jaunty jewel
#

also i left some comments in #2483

primal torrent
primal torrent
#

is there a reason twilight takes an str arg for emoji image and not just bytes?

wild marlin
#

Does it not have to be base64 encoded?

primal torrent
#

if it does then the twilight example for it is very bad

#

cause it gives 0 indication of that

#

this is the docs example

image.png
wild marlin
#

That should probably be fixed along with information that it should be 128^2

primal torrent
#

does it neven need to be that size? dev portal says recomended but not required

#

guess it's time to play around with it a bit

primal torrent
#

ok so it does not need to be that specific resolution, and you need to properly prefix it

wild marlin
#

But the size limit is correct?

#

Though a good question is if it is the base64 size or the raw file size.

primal torrent
#

also fixed the name, as emoji can't have spaces in their name

#

oh fun, clippy failing on something completely unrelated...

wild marlin
#

Probably because rust was updated yesterday

primal torrent
#

gona finish this tui thing first and then look at fixing that. it's a very simple one about unchecked subtraction

eternal owl
#

is the gateway meant to not run with default features?

wild marlin
eternal owl
#

i understand the thought process, but simultaneously find that disappointing for the user experience. the twilight-gateway readme example doesn't run, i would at least suggest making that something users can copy-paste

jaunty jewel
#

Have we seen any endpoint from the Discord API returning maps yet

wild marlin
#

I think there are a couple of models with maps maybe

#

Can't remember if any of them are from http enfpointsthough

wild marlin
jaunty jewel
karmic aurora
#

There's no need for a MapBody marker, just use HashMap<Id<RoleMarker>, u64> directly

wild marlin
jaunty jewel
#

Filed #2489, needs second review

jaunty jewel
#

@wild marlin #2488 needs fmt and test fixes

wild marlin
#

I think I will try and get some new versions out when 2490 is merged

jaunty jewel
#

2483 also needs second review

karmic aurora
#

#2487 would be nice to get in together with 2490 for next time

jaunty jewel
#

approved

jaunty jewel
#

@wild marlin addressed your review comment

wild marlin
#

NOTE: We reccomend everyone to update to this version as soon as
possible as the ratelimiter may panic in some cases on version 0.17.0.

#

I will publish the updates now

#

I now know that the workspace publish does not work unless you have updated all crates

#

but you can just have multiple -p crate-name

primal torrent
#

can you supress the embeds in #announcements ? it's an entire screen of just the same github embed

wild marlin
#

I don't know if supress embeds also means they will be suppresed where they get published to so I rather wanted to be on the safe sidte

primal torrent
#

there should also just be an x button to supress them all if you're a mod or its your message

#

yeah that carries over

wild marlin
#

Ahh okay

#

Spelling blobsved

#

Okay everything should be published now

primal torrent
#

thanks for pushing that out so fast

wild marlin
#

No problem, I kinda wanted a fix for such a bug out quite fast

primal torrent
#

yeah that's a massive one

#

looking at the file there are a few other unwraps we probably could improve in time or at the least turn into more useful expects. some other areas of the code have the same thing

#

but that's a later problem

karmic aurora
#

I will update the http proxy as well

karmic aurora
wild marlin
primal torrent
#

Like just having the route and some basic state can help a lot so users don't need to mess with debug outputs to get basic diagnostics

karmic aurora
#

even printing the entire state may not be enough as it's just a single snapshot. you really need to track state updates as they happen over time

primal torrent
#

Sure, but knowing the endpoint that finally blew it up already can helo narrow it down a lot

karmic aurora
#

It would not have helped in this case. What narrowed this down was the line number. Then I just ran through different code paths until I found the error. Knowing one piece of data is indifferent unless you know all pieces of data (over time, as provided by the debug events)

jaunty jewel
jaunty jewel
#

^^

wild marlin
#

Tbh I am not sure

#

Like we probably should in some way but they are, a bit odd compared to the normal stuff

jaunty jewel
#

Agree

karmic aurora
#

Re 2493: should this target main if it currently always fails to deserialize?

primal torrent
#

I think so, its imo a bugfix more then a breaking change

jaunty jewel
karmic aurora
#

Our format CI job skips our readme and book examples and some of it is therefore incorrectly formatted

wild marlin
#

@karmic aurora Would a LazyLock not work better for those examples

karmic aurora
#

It's designed to support data fetched on startup, e.g. the bot application/user id, but no, it's not required in these simpler cases

#

I'm actually updating the lavalink-basic-bot example right now and lavalink requires the bot user id:

let http = HttpClient::new(token.clone());
let user = http.current_user().await?.model().await?;
let hyper = HyperClient::builder(TokioExecutor::new()).build_http();
let lavalink = Lavalink::new(user.id, 1);
lavalink.add(lavalink_host, lavalink_auth).await?;
let standby = Standby::new();
context::initialize(http, hyper, lavalink, standby);
wild marlin
#

(the one that updates lavalink)

karmic aurora
#

no, but the public api is mostly the same right?

wild marlin
#

I think so

#

I think I might try and set it up today to test the new lavalink bindings

wild marlin
#

I have actually tested it now and everything seems to work

karmic aurora
#

Updating the examples got me thinking about provides the implementations instead. Not sure whether it would belong in a support library/twilight-util/or a new twilight library

wild marlin
#

Implementation of what?

karmic aurora
#

The set up (I'm unsure about the api)

wild marlin
#

Ahh so like a framework-lite to just set up the most necessary things?

karmic aurora
#

Yes, I think a lot of users set up twilight in a non-ideal way

wild marlin
#

Yeah that is probably true

#

I think we could have a new crate for that, maybe twilight-init

primal torrent
#

an actual demo project or template can also help a lot and might even be more useful

karmic aurora
#

A template would be great, but I don't know how we should distribute it

primal torrent
#

template repo people can just use to bootstrap?

wild marlin
karmic aurora
#

btw erk I'm planning to update the lavalink example in another PR, the one currently live is feature complete

#

I guess the github native template feature is easiest, though I've not seen it used that much

primal torrent
#

pretty sure cargo generate can use a github repo as source as well so shouldn't be hard to do one repo for both

jaunty jewel
jaunty jewel
primal torrent
#

flexible is good, but twilight is too large to find the basic starting point yourself sometimes

karmic aurora
#

I would say that it's impossible unless you are well versed with tokio and async rust

wild marlin
#

And probably also the discord api to some degree

karmic aurora
#

@wild marlin I won't be reviewing the lavalink v4 PR, but you can admin merge it

silent marsh
#

Yeah I agree, just merge it

#

And then let's start the process of removing it

primal torrent
#

maybe first see if someone wants to take it over and how? then we could do a final release with a warning on starup baked in that it's no longer maintained and where to find the new one

#

or at build time

karmic aurora
#

note that it should target main enough though it's breaking due to 0.15 being the latest lavalink release

wild marlin
#

Okay I will merge it and say that we will probably split it out into its own repo next

#

🎉

#

Merged

wild marlin
#

tbh I am not a big fan of a global context, but I think I am in the minority there

#

I rather just have a struct I pass around to everywhere

jaunty jewel
#

but anyway

karmic aurora
#

rebased the lavalink example

karmic aurora
wild marlin
#

I rather just put it in a Arc tbh

#

I guess that adds a bit of overhead but I am not sure it is measureable compred to other things here

#

Like the difference between a atomic load and a atomic increment can't really be that much here

#

or I guess a arc is 2

karmic aurora
#

it's more about not having to .clone()

#

arc is a non-atomic ptr + atomic counter so there's only 1

wild marlin
#

Yeah but it has to check if it is dead before a increment right?

karmic aurora
#

ahh there's a weak and a strong count never mind

#

502 bad gateway

wild marlin
#

Actually it is only one increment

wild marlin
# karmic aurora 502 bad gateway 
async fn runner(mut shard: Shard, ctx: Context) {
    while let Some(item) = shard.next_event(EVENT_FLAGS).await {
        let event = match item {
            Ok(event) => event,
            Err(source) => {
                tracing::warn!(?source, "error receiving event");

                continue;
            }
        };

        tokio::spawn({
            let ctx = ctx.clone();
            async move {
                handle_event(event, ctx).await;
            }
        });
    }
}
#

If you have an Arc the strong count can be assume to be at least 1 so you don't need to check

karmic aurora
#

true

#

I think I've done repeated clones to parallelize (spawn()) inside of an event handler, but I don't know why I would have done that

wild marlin
#

I might clone it to give it to the runners as well, but that is a one time cost so that is free

wild marlin
karmic aurora
wild marlin
#

odd

#

I am loading it fine from here

karmic aurora
wild marlin
karmic aurora
karmic aurora
#

Set up basic framework, opened a PR to import the gateway example as a start

#

I'm thinking that we may prompt to include some administrative commands (inspired by what @primal torrent has), but I'm otherwise open to suggestions

primal torrent
#

prob something that spawns some shards into their own workers, has a simple central state with sends to the shards and that updates an in memory cache on receiving events?

#

maybe with an async spawning off for further handling

karmic aurora
#

The gateway example already includes dispatcher and event handler tasks. What central state would you have?

maybe with an async spawning off for further handling

In addition to the event handler? What should happen there?

primal torrent
#

i think the template should be more then the examples

karmic aurora
#

As a long term goal it would also be cool to provide a multi-process template

primal torrent
#

after updating the cache

karmic aurora
#

That's the plan, but the template needs to do something concretely

#

I call the event loop tasks dispatcher tasks, so that's already implemented

primal torrent
#

we could have some basic ping command (interaction, let's avoid text based commands)

#

i'm rewriting gearbot in rust so maybe some very basic component parts like an user info command can (mostly) be reused

#

maybe we could also do the "assign random role on join" thing as demo?

#

and maybe like a simple command to give a count how many members each role has to show interacting with the cache

karmic aurora
#

There's two things we could do:

  1. show off how do to things in Twilight (but substitute your own logic)
  2. provide useful utilities
primal torrent
#

i think twilight could use a bit of both

karmic aurora
#

I think 2. offers a better developer experience for the templates, so I prefer that as much as possible

primal torrent
#

right now getting started is quite a bit of looking and digging through code

#

the template could show off how to use those utilities

karmic aurora
#

yeah, showing off how to do something by providing an actual useful utility is the best of both worlds

#

could you list what bot management stuff you have defined? I'm thinking that we can implement them as slash commands

primal torrent
#

you mean the whole manager thing i'm building?

#

if so, that won't really work here cause if you kill the shard the interactions from that guild (or shard 0 for dms) end up on you lose control

karmic aurora
#

We'd not kill the shard, but restart it. That should work, no?

primal torrent
#

could work

#

not sure how much of what i'm building would be reusable though since mine works entirely on grpc and the separate manager also holds the in memory queue now to coordinate shard starts

karmic aurora
#

I don't want to reuse your code (that creates licensing issues), but I just want to know what stuff you've implemented. So far I know about the ability to restart shards

#

I'll do the actual implementation

primal torrent
#

it's wip but my restart is more a "tell bot to kill the shard, tell another instance to make the shard"

#

could in theory be the same instance though

karmic aurora
#

The template will start by being single process, so it'll be a bit simpler

primal torrent
#

by using a sender that just sends to the event loop actor a message that terminates the loop and cancels the cancellation token it passes to all handlers and such to link to if they wana do further spawning off or long running tasks

#

one thing on my todo is also to on shutdown also not just kill everyting, but simply send that message to all shards, and put a flag on the bot that it's shutting down

karmic aurora
primal torrent
#

only once all shards are down and that flag is set would that then cancel the "global" cancellation token to shut down all supporting tasks (like manager reporting, grpc server and all those)

#

speaking of supporting things i plan to make that might be useful as twilight util thing: prometheus or other metrics reporting

#

will be a bit before i get to it but might be something to consider if we can't make a generic shard "wrapper" for this

#

that does the tracking and has an easy to setup function to call to setup and start the webserver to serve the data to a scraper

#

feels like something that should be possible and i wouldn't mind spending some time on later to make more generally available

karmic aurora
#

That would be useful. I'm not too familiar with the metrics ecosystem though

primal torrent
#

would need to look into what the modern rust options are, but iirc there are some crates that can do metrics in general and some that can do prometheus

#

need to be really mindful and careful not to have a big performance impact (like might need an actor to avoid something locking), but almost every bot can benifit from basic statistics of at least what it receives. maybe even if we can somehow time how long your event handling (time till drop?) takes

karmic aurora
#

See I'm not even familiar with prometheus

primal torrent
#

you know grafana?

#

prometheus is just a datasource for it. a database optimized for time series data. basicaly points of data over time

karmic aurora
#

I've heard about it, same with prometheus, but I've not used it

primal torrent
#

it can be an amazing monitoring tool

#

like we could track how many of each event type you receive on every shard and feed that to prometheus and you can then have pretty graphs of it over time

karmic aurora
primal torrent
#

if your bot suddenly struggles and you see a 4x increase of a specific event type around that time you got a pretty good starting point to look

#

prometheus gets data from the app by scraping it so you need to follow the right data format

#

there are also other possible datasources line an influx db you can use or many others

#

prometheus tends to be the easiest for a decoupled system though. the application only needs to expose a rest endpoint that provides the data and not be aware of any credentials or maintain db connections

karmic aurora
primal torrent
#

iirc there are some rust crates that do that but would need to look into that

#

they do have downsides though you can generally only use what every single data source supports

karmic aurora
#

I would expect them to offer the same feature set, since they all map to grafana?

primal torrent
#

but we can revisit that once i finish my manager and start working on stuff like that. would need to try some libs and see what options they got

primal torrent
#

but the reason there are different sources is they do things differently and got different features

#

prometheus is for example just a set of datapoints is scrapes every x interval. they got labels and that's it. there are a few different types

#

but with influxdb, or even postgres sources what you can do is query data tables (that can also be filled retroactively then) that return a set of datapoints in time

#

heck promethues now can even do logs over time (loki data source) and distributed traces (zipkin datasource)

karmic aurora
#

okay sounds great (does not quite understand non-prometheus datasources and why they might be used)

#

thank you for your explanation btw

sage cloud
primal torrent
#

if shit suddenly slows down and it shows a "starting heavy background task x" right at that time

karmic aurora
#

souds useful, and a bit like sentry (I've also not used sentry)

primal torrent
#

little bit, but without the stack traces or grouping errors

sage cloud
primal torrent
#

twilight already provides a ready to run container for the server side

karmic aurora
#

further template discussion in #1455998449077063690

karmic aurora
#

Does anyone feel like reviewing the template PRs? 🥺 (#3 is now dependent upon #5, so please take a look at it first)

primal torrent
#

currently trying to wrap up the shard coordination in my manager project but will try to find some time tomorrow or next week depending on how that goes

jaunty jewel
#

let me take a look

wild marlin
#

tbh I am not really sure if that should be in the general template

#

I feel like it complicates it a lot

jaunty jewel
#

i see your point

karmic aurora
#

Discuss in #1455998449077063690 please

rain niche
karmic aurora
#

You are likely the first to notice. Our API is very large so these things are bound to happen

#

it should of course be fixed

rain niche
#

In which direction though: Position.position to i32, or Channel.position to u64?

eternal owl
karmic aurora
#

but we could unify on a integer size: 32 or 64. I think 32 should be enough, it's unlikely to see more than 2/4 million channels

rain niche
rain niche
#

But then, if the API does accepts and returns negative numbers, twilight won't be able to properly interpret those. I'll check what other libraries are doing.

karmic aurora
#

we'd fail to deserialize such responses, which is really bad. it sounds like we have to keep it signed

#

well, we could write a custom deserializer, but that complicates maintenance (our internal api)

eternal owl
rain niche
karmic aurora
#

update has minimum: 0, but data type has no such annotation, so it seems to me that they may send negative integers

rain niche
karmic aurora
eternal owl
#

oh, I linked to the wrong one. that's linking to the template channel data type, this is the GuildChannelResponse. and yeah, it also isn't bounded to a minimum of 0. so I'd say it's still possible to return negative channels. unlikely, but possible, and not restricted by the spec

rain niche
#

The CreateGuildChannel seems to be bound to 0.

eternal owl
#

indeed. upserts should be unsigned, reads should be signed

rain niche
#

It's pretty consistent. We can assume that if setting positions force them to be positive int32s, we cannot get a negative one from the API, even though it's not specified as such. Still, that would require a survey for confirmation.

#

Regardless, to be compliant with Discord's API, Twilight would have to use an i32 with a lowerbound set to 0.

karmic aurora
#

We should migrate to it

jaunty jewel
#

not yet

karmic aurora
#

Also is Discord still prone to disabling HTTP 2? It would be nice to remove HTTP 1.1 support (unless behind a proxy)

wild marlin
#

I think it might happen if some new dos issue is found

karmic aurora
#

That's too bad, we will have to wait a while longer until they figure out such new technology

wild marlin
#

We could on the other hand add http3 support because cloudflare also proxies that

karmic aurora
#

I don't think that's live in hyper

rain niche
#

Is it necessary to drop HTTP 1.1 support?
How much work would that be to maintain it?

rain niche
#

Regardless, it seems safe to change all the position types to i32 and either:

  • Write a late-check when requesting to prevent sending position < 0
  • Write a type that ensure the position is > 0 (NonNegative<i32>?)
wild marlin
silent marsh
#

just a heads up: I will probably be raising the tokio-websockets MSRV to 1.93 sometime rather soon after release, if twilight wants to stick to 1.89 I'm happy to maintain two versions for 1-2 compiler releases

wild marlin
#

I personally don't have strong opinions about sticking to msrv

#

What's in 1.93

silent marsh
#

Stable s390x vectors

#

At least that's what I'm currently eyeing at

#

Some of the stabilized APIs also look quite interesting, though I don't think I'd be able to make use of them

wild marlin
wild marlin
#

@karmic aurora i spotted one issue with the merge queue, look in #github-log

karmic aurora
#

It would be nice to filter branches starting with gh-readonly-queue/

#

I'm not sure I have access to the webhook

karmic aurora
#

Hmm it's not possible to filter the tags on GitHub's end. But we could stop all branch events. The pull request events are the useful ones in my opinion

wild marlin
#

Yeah I have been annoyed by it before as well, it cannot really do anything you want

swift gull
#

GitHub doesn't even support filtering pre-releases when watching project releases. I get tons of -rc release notes for Mattermost every month

wild marlin
#

Why even make it possible to mark a release as a pre-release

karmic aurora
wild marlin
# 
ERROR Invalid configuration file
    Caused by: TOML parse error at line 5, column 1
  |
5 | multilingual = true
  | ^^^^^^^^^^^^
unknown field `multilingual`, expected one of `title`, `authors`, `description`, `src`, `language`, `text-direction`

Process completed with exit code 101.

Unknown field is apparently a hard error

karmic aurora
#

that was very quick

jaunty jewel
wild marlin
#

I will see if I can get a release together one of the coming days (probably Saturday)

jaunty jewel
#

I'll try to work on the new invite endpoints asap

karmic aurora
#

#2502 and #2506 are large, but I'd love for them to make it in time

jaunty jewel
#

approved one of them - will look into the other one

karmic aurora
#

I want the twilight-http dependency removed from twilight-gateway to improve compile times (removes the create_recommended function). The template shows that calling GET /gateway/bot is already a soft requirement to properly setup the gateway queue. Looking through our examples, #loc will remain the same

#

I'm also looking to deprecate create_bucket and create_iterator, as the template has shown that these do not compose nicely with session resumption. They would be replaced by fn bucket(id: u16, buckets: u16, total: u32) -> impl Iterator<Item = ShardId> (implementation), where you would be responsible to zip shard ids and configs.

karmic aurora
karmic aurora
#

The aforementioned follow-up pr to #2511 will deprecate create_recommended, so it's soft-dependent upon #2512 as well. I will create the pr after #2511 or #2512 is merged

karmic aurora
#

@wild marlin would you like 1) all libraries 2) or dependent libraries to link to the template? I'm not convinced any should, but it may boost discoverability, which I'm for. twilight-gateway is otherwise a bit special since it's the bot "core"

wild marlin
#

I think it makes sense to link it from all libraries which is show off in the template (so not stuff like rate limiting) since it is a entrypoint in a way

karmic aurora
#

That would be the gateway, http, model, and util. But only the gateway and http can be considered bot entrypoints

wild marlin
#

I would be fine with only having it in those two

wild marlin
#

I have updated the next branch

#

I have started to look at splitting up http and see if it can be done in some nice way

wild marlin
#

I started with trying to fix up the routing a bit

#

Trying to figure out a nice way to build up the paths so I can move that out to every request instead of it being in the routing.rs file

karmic aurora
#

also please take a look at template#10 if you are experienced with large bots

wild marlin
#

@karmic aurora could the app_id not just be saved in the context?

#

At least that is what I do

karmic aurora
#

I have thought about that [1] and I feel like we should at that point remove the whole concept of the InteractionClient and let the http client save the application id (and expose the methods directly). Adding such a context method exposes that the twilight_http api is awkward, imo.

#

idk, I could be swayed either way

wild marlin
#

I personally think that the InteractionClient is a pretty nice abstraction because it can be used in that way, otherwise you end up with a lot of something.application_idin your code

#

Though I also kinda agree, so I am not sure what the best solution is.

#

I just don't think it is to find the application_id every time you need it from somewhere.

karmic aurora
#

done

karmic aurora
#

There's a race condition in http-proxy's expiring_lru where an entry may be accessed at the same time that it's scheduled to be removed. This is an invalid delay_task state that results in a panic. Fixing the logic error requires synchronization (Mutex) or we could just switch to a single-threaded runtime.

// [delay_task]
debug!("Refreshing entry in ratelimiter decay queue");
// This will panic if the key is not present, therefore
// we check that in the calling end
queue.reset(&key, expiration);

// [request_task]
let entry = self.inner.get(key)?;
_ = self.decay_tx.send(TimerUpdate::Refresh {
    key: entry.decay_key,
});

But the reason I bring all this up to discussion is because I feel like a tlru is a bad fit. Would a static token: rate_limiter map not be preferable to a dynamic one? Also, this would be a breaking change but we don't tag containers with semver labels

silent marsh
#

or we could just switch to a single-threaded runtime
This is probably not a bad idea, but hard to tell

karmic aurora
#

I suppose we could announce a future breaking change through deprecation + a (logged) warning

silent marsh
#

Would a static token: rate_limiter map not be preferable to a dynamic one?
I'm pretty sure there was a reason why that wasn't done

#

I don't remember what I had in mind back then though

karmic aurora
#

My idea is for DISCORD_TOKEN to be variadic (token1,token2) and either rejecting requests without a matching token or assigning the first token when no token was provided in the request

silent marsh
#

Since you're then authenticating with a bearer token that's only valid temporarily and cannot be known upfront

karmic aurora
#

are those even rate limited?

silent marsh
#

Not sure

#

Likely yes

karmic aurora
#

regardless, should you run oauth through http-proxy? it's not a featured usage and its support is, I feel, accidental

silent marsh
#

It is something people have done before

#

and especially since we now have ratelimiting for unknown routes we do "support" it

#

I see no reason to break that usecase

karmic aurora
#

the /oauth2/token route does not set/require a token

#

/oauth2/@me is a rate limited and takes a bearer token

#

Okay, so we should support bearer tokens and they have to be dynamic. But rather than a tlru, bot tokens should be static and bearer tokens should be removed on /oauth2/token/revoke requests. That should not require breaking changes

silent marsh
#

huh, but not all OAuth tokens are manually revoked, most just expire

karmic aurora
#

I'm exposing my unfamiliarity with oauth

#

Okay, the TLRU is staying (lol). I still think we should work on bearer tokens a bit, cause our support feels a bit accidental or magic to me

silent marsh
#

We used to support them in twilight, but removed it for some reason

#

does not remember why

coral storm
#

Hey everyone, for a separate project I was working on I wanted to use the checkbox components discord added relatively recently but saw that they weren't implemented yet. I cloned the library and added them myself, and it all seems to be working fine, but I haven't done all the documentation or set up all the tests. Would you be interested in a PR for this?

wild marlin
wild marlin
#

@coral storm Sent a review your way

karmic aurora
#

We should replace pretty much every allow with expect

wild marlin
#

Wait does it work when you do it at module level as well?

#

I did not know that lol

#

I guess it makes sense though

wild marlin
#

@karmic aurora Have I missed any pr you want a review of?

karmic aurora
#

#2499 would be nice

#

#2502 as well

wild marlin
#

I had just looked through 2502 blobsved forgot comment anything

karmic aurora
#

#2500 is now also unblocked

wild marlin
#

Like you clear the various maps in shutdown, but the entries can also remove themselves

karmic aurora
#

You mean that the cleanup function could just be shutdown?

wild marlin
#

Yeah I guess so

#

And a way to stop new things to be added

karmic aurora
#

wdym? shutdown has to be checked before and after action() runs

wild marlin
karmic aurora
#

I agree

wild marlin
#

Also maybe add a way to check if it is shutdown

#

just for good measures

#

like just pub fn is_shutdown(&self) -> bool { self.shutdown.store(true, Ordering::Relaxed) }

karmic aurora
#

you mean load :)

#

pushed the afforementioned changes

#

thanks for all the reviews!

#

I think that's it

wild marlin
karmic aurora
#

Ahh

karmic aurora
#

new CI warning: 'haswell' is not a recognized processor for this target (ignoring processor)

#

I think we can drop the -C target-cpu directive entirely. I've not seen another project specify it, so the default should work

karmic aurora
#

I reopened template#4 (multi-process template), but I am planning to resolve its use case through persisting the cache instead. Given a 5 minute resume time, even a 100 MB/s HDD could support a 15 GB cache. It just requires a cache import/export API. Does anybody object to this?

#

FYI, I think a cache import/export API should be added regardless

karmic aurora
wild marlin
#

tbh as long as it does not make the cache more annoying to update I am fine

#

imo its already one of the less nice places to update when there is new fields and such

karmic aurora
#

I don't think anything more than #[derive(Deserialize, Serialize)] is necessary on the models

jaunty jewel
#

Would love some pointers to implement data urls

#

esp. for creating soundboard sounds

south forum
#

hi I'm currently working on a pr to implement the Search Guild Messages endpoint, just to confirm there is nobody else working on a conflicting PR?

jaunty jewel
#

go ahead

south forum
#

Awesome thanks

karmic aurora
wild marlin
#

Its a bit annoying that these two does not just both return either a list or a struct with a list in it...

image.png
#

Like I don't care much, but choose one or the other.

wild marlin
#

Hmm I have to figure out how soundboard permissions work, I thought I had given them to @sharp inlet, but it did not seem to do anything.

wild marlin
#

Ah there is just a different permission for creating and modifying/deleting them for some reason

#

I have most of it implemented so I will probably pr today or tomorrow since I am running low on battery now.

wild marlin
#

I think that is all the http stuff maybe it needs a bit of validation

#

There is apparently also a gateway event which I need to look into