#topgg-open-source
1 messages Β· Page 4 of 1

From what I see here, @drowsy thistle is already doing this: https://github.com/Top-gg-Community/node-sdk/pull/92
one day it can be merged but null has the tendency to change the entire codebase in one commit and then i can't properly review it
null's code is interesting
I was only going to change one thing in this code, but I wanted to know what is returned in /v1/projects/@me/commands?
...i no longer do that
yeah, the last update was in October
can you review my pull requests?
please link them
they are in pins but sure
https://github.com/Top-gg-Community/python-sdk/pull/89[MERGED]https://github.com/Top-gg-Community/python-sdk/pull/90[MERGED]- https://github.com/Top-gg-Community/node-sdk/pull/92
https://github.com/Top-gg-Community/node-sdk/pull/94[MERGED]https://github.com/Top-gg-Community/node-sdk/pull/95[CLOSED: Its changes added to #92]- https://github.com/Top-gg-Community/rust-sdk/pull/33
- https://github.com/Top-gg-Community/rust-sdk/pull/34
- https://github.com/Top-gg-Community/python-sdk/pull/91
- https://github.com/Top-gg-Community/python-sdk/pull/92
- https://github.com/Top-gg-Community/python-sdk/pull/93
- https://github.com/Top-gg-Community/dotnet-sdk/pull/30
https://github.com/Top-gg-Community/dotnet-sdk/pull/32[MERGED]- https://github.com/Top-gg-Community/dotnet-sdk/pull/33
https://github.com/Top-gg-Community/lua-sdk/pull/4[Draft]https://github.com/Top-gg-Community/java-sdk/pull/24[CLOSED: Its changes added to #25]- https://github.com/Top-gg-Community/java-sdk/pull/25
- https://github.com/Top-gg-Community/java-sdk/pull/26
- https://github.com/Top-gg-Community/java-sdk/pull/27
- https://github.com/Top-gg-Community/go-sdk/pull/15
- https://github.com/Top-gg-Community/go-sdk/pull/16
https://github.com/Top-gg-Community/ruby-sdk/pull/1[Draft]https://github.com/Top-gg-Community/php-sdk/pull/5[Draft]https://github.com/top-gg/docs/pull/101[Draft]
<@&1324896486130974720> it would also be nice to have some other people review here pls
yes please
good luck!
dw some of the pull requests have ~100 diffs
please let me know if you have any concerns
one second, i'm working on python-sdk's #90
Did we ever get perms setup for staff to do reviews?
responded to your comment in https://github.com/Top-gg-Community/python-sdk/pull/89
also i forgot to update the license year
oops
one sec
nvm i'm getting sidetracked, i'll just do it later
let me see if i could make the coverage 100%
I'd prefer if we could focus on getting things merged instead
also i've responded to your comment in https://github.com/Top-gg-Community/python-sdk/pull/90
also, can https://github.com/Top-gg-Community/python-sdk/pull/89 be merged now? (i've also responded to your comment there.)
ya that's fine, for future reference i'd prefer using https://spdx.dev/ 's standards for external sdks. Most tools that maintain licensing for enterprise (the only reason people add licenses on files) rely on this standard.
noted noted
i've also responded to your comment in #90
alright thanks for merging it π«Ά
from now anyone can also refer to this list if anyone wants to see my pull requests
Hello <@&844267965742186619> :)
We have an openapi spec now. https://top.gg/api/v1/openapi/spec.json
prolly some thing are still a bit wonky
e.g. integer(int64) is parsed as strings for our api
Neat
there's also a swagger endpoint (https://top.gg/api/v1/swagger/v1/swagger.json) with more information around status codes
hosted UI will come later

me when the versioned api is versioned
big w
I need a bot that notifies me when someone new joins a server
cool, good luck with that
lol
Did u check if its available?
I mean every mod bot has welcome message
Just turn it into private channel and make custom message saying this user joined server

Yeah that's incredibly easy to find.
for the Python SDK's v0 pull request: I've bumped the license year and replaced MIT license comments with SPDX compliant license comments -> https://github.com/null8626/python-sdk/commit/45c8503de006b374e2f21027596e66e1c806626f

planning to do this to the SDKs where i am the core maintainer of -- as I'm waiting for further review
done for the C++ and Rust SDK 
I'm a bit confused here, I see a bunch of reformatting on the project and then there's feature updates here and also license formatting changes? All these together makes it impossible to review
oh, sorry! will turn this to a separate pull request 
changes reverted for the Python SDK 
changes reverted for the C++ SDK 
changes reverted for the Rust SDK 
the rest of the SDKs simply had a bumped license year, these changes shouldn't interfere that much!
if you want me to split the prs further or rework some of them, please let me know!
fixed conflicts in the Python SDK pull request 
license-related changes has been reverted in the Python SDK pull request π this should make reviewing it a bit less tiresome
reminder: if anyone wants to review the Rust SDK pull request, please use this diff instead, as it covers my previous changes and the existing pull request that reverts it: https://github.com/Top-gg-Community/rust-sdk/compare/a7ed300e4a8ad62b4823aab84c2c59fd7bcaf955...null8626:rust-sdk:split/revert-v2
GitHub workflows and Eslint-related changes have been reverted in the Node SDK pull request 
these changes will go in another pull request
thank you so much for your comments Veld! β€οΈ
now the Node SDK pull request can be merged!

cc @tiny shard
Lmk if you want me to check something
review the ones you are best suited on!
was this addressed? https://github.com/Top-gg-Community/node-sdk/pull/92/changes#r2345709137
left 1 comment as well
build is also failing it seems
addressed and resolved
i reverted my github actions and eslint changes because i thought they would be irrelevant to the pull request's scope
maybe i can bring them back if you want?
no need for config changes but the eslint version was bumped causing an error in even running it
id revert the eslint version upgrade and make a separate pr updating it if you want it on latest
sure
addressed and resolved
the response field was still removed
sure, but i'm not sure how i would do it for something like this
do i just leave it as undefined?
resolved
using undici's MockAgent intercept() here is certainly possible 
wait nevermind, that could be a bad idea if i use setGlobalDispatcher
i could create a mock β¨β¨β¨Dispatcher.ResponseDataβ©β©β© but that would be a hacky solution
but if i leave it as β¨β¨undefinedβ©β©, the user would get no error message as a clear reason
nevermind, i'll just revert it altogether
reverted
the pull request can be re-reviewed again, so sorry guys! 
this (and my other) pull requests have been pretty messy, i'll promise not to repeat my mistakes
probably just create another error class that can be moved to in a major version
is erroring out without an api call not a goal?
well it used to just warn the user about the deprecation via β¨β¨console.warnβ©β©
we just need to convey to the users that the endpoint is deprecated
but veld said that we should prioritize on adapting it to v1 over things getting deprecated in v0 (which has been subsided by v1) so it's not that big of a deal
<@&412346069675147264> https://github.com/top-gg/webhooks-v2-nodejs-example
reading back in that PR, I'd probably be okay with just bumping a major version and deleting the v0 api from the sdk.
If you use the old API, just like don't update.
But I feel like that's my thoughts since we're not really anywhere closer to releasing a library version, and the v1 api is basically going to be on feature parity as v0 in a week orso...
Previously I felt that if people wanted to gradually adopt the new API they could do it this way, but I don't think that makes sense anymore in terms of timing
Anyways, webhooks v2 public API is set in stone now. Above is a working example from my testing, figured I'd share so people can prepare or something π«‘
but you've been saying not to create breaking changes 
Go for it
alright
Iβll add the v1 API stats push endpoint later this week
Then the API v1 can replace all common use cases
thanks, will work on it
as soon as i can
i have not slept tho, maybe soon
Go sleep π₯Ή
Fr
so i do this, right? (fyi lints and tests run fine with this)
that's a lot of deletions! π
yup only v1 should be left in the 4.0 version
π«‘
noted!
that being said, before dealing with this v1 pr, regarding the previous merged v0 pr (#91, https://github.com/Top-gg-Community/node-sdk/commit/4d7cc93c4a7f0104fac5c07bb88e5218c54c1f4c), berry, do you want to bump a minor semver and publish it to npm?
the pull request regarding the Node SDK's eslint changes is here: https://github.com/Top-gg-Community/node-sdk/pull/94
conversational reference: #topgg-open-source message
merged

should we drop support for node 18 as it's no longer supported?
this would be done by making the github workflows test from version 20, and make .nvmrc 20.0.0
@tiny shard @dense barn what do you think?
mmmm. only concern would be to create an easier upgrade path, changing a package version is easier than changing a node version
iβm okay with it but if weβre trying to keep spirit with the goal it might be worth continuing support for just the one release first
is there a point?
thought we would release a newer version for the users who still use v0
before we release the major version that deletes the v0 features
I think that would've been alright 4-5 months ago, but very little value now imo
alright!
opened the other one: https://github.com/Top-gg-Community/node-sdk/pull/95
this one is fairly straightforward
hm, it seems that newer versions of undici does not work with Node.js v18
@tiny shard #95 is approved by veld, can you look at it?
in the meantime, i'll work on implementing more v1 features to #92 
crap, i forgot to add discord-api-types
i'll just add it to #92
i'll handle the conflicts
implemented GET /v1/projects/@me on the Node SDK 
time to work on the new webhook approach!
done! what do you guys think? https://github.com/null8626/node-sdk/compare/bb1a39ef7d42026c47f97fd7373b14b41cbb21e1..master
oh by the way, should we continue to support the /api/weekend endpoint? it's not attached to any API version that I know of
bumped to the next major version, removed all v0 features, and implemented GET /v1/projects/@me on the .NET SDK 
will update the webhook pull request 
i think both the Node and .NET SDK are now virtually done! 
i'll work on the other SDKs when I am free haha, sorry! I have final exams next week 
no, it's not even documented. the new vote endpoint gives you the weight of each vote made.
people would never need to call "/api/weekend" because of this
v1 api required bearer btw
An official module for interacting with the Top.gg API - Comparing bb1a39ef7d42026c47f97fd7373b14b41cbb21e1..master Β· null8626/node-sdk
oh alright, mb mb
alright!
check out something like this: https://github.com/OpenAPITools/openapi-generator-cli
A node package wrapper for https://github.com/OpenAPITools/openapi-generator - OpenAPITools/openapi-generator-cli
swagger also is able to do it: https://swagger.io/tools/swagger-codegen/
Codegen simplifies your build process by generating server stubs and client SDKs for any API defined with the OpenAPI specification. Download Codegen today.
could generate crud clients for all languages iirc
that way actual work on SDKs can be done by working on integrations like webhook listeners, specific library integrations etc
done!
done!
so all of these are nullable, yes?
that means i will ?: virtually everything
like all of these properties are nullable, even though in docs.top.gg many of them are not
Ah no theyre not all optional
in spec.json
even if non-nullable properties don't have it, there's no way openapi-generator-cli would mark every property as nullable
thanks for flagging, will fix
well tbh if the ones in docs.top.gg are correct, then i don't need to change anything
Python SDK pull request now closed, working on a major pull request that adds v1 support but deletes v0 features
6 other pull requests also closed under the same reason
The Java SDK's webhooks pull request now uses the new authorization approach!

refactored the .NET SDK pull request on it as well
I'll work on the rest when I am free. schedule is crazy packed for the 10 days π
I've made existing WIP pull requests draft 
fixed webhook link address in docs
https://github.com/top-gg/docs/pull/106
awesome!
but why add yarn to package.json?
i'm okay with it but it's a bit odd
I just installed it
any now anyone else who pulls that will also have it
its needed to run the server
well I better hope so
btw william, if you have time, can you review some of my pull requests?
this still accurate?
alrught
left comments on them, they look good to me don't have perms to actually review them
thank you soo muchh williamm!!!
you took the time and that's all that matters
love u π«Ά
me : lgtm
production : nuh-uh π
tests: yuh uh
that's the only serving grace tbf
I'll address the .NET SDK comments when I am free as I am undergoing final exams right now, thanks Veld!
I'll also do the same to other webhook pull requests, sorry Veld! π I did not know about the payload changes as well
every pull request updating it will be drafted for now.
I promise I'll work on them next week. 
Hi
hello!
you can look it up in the website, also this is not the correct channel to ask it π
@dense barn hey uhm
by the way I have addressed your .NET SDK's comments. what do you think? -> https://github.com/null8626/dotnet-sdk/compare/f787f56961c53e66d17d8cdecabc0e3019c2919b..split/webhooks?diff=split&w
Is that comic sans ms 
yes
currently working on updating the Rust SDK 
thx

ah, ur asking for votes from right now. so there aren't any votes at all.
will fix the bug that it 500s tho
π«‘
fix pushed, should be working in approx 10 minutes
added getVotes to the Node SDK
the pull request can be reviewed now!
I think I can safely say that the Node SDK is now complete π
post when?
the Rust SDK is complete from an API standpoint. all it needs now is to test its updated webhooks!
i am really tired, so i am going to bed right now π will work on the rest in a few days
also from now on i'm gonna work on one SDK per day so i don't get burned out so quickly, sorry everyone! 
the Rust SDK is now complete!
these are pretty big diffs π i'll split it into two pull requests
oops, forgot to undraft it yesterday π
now it can be reviewed! 
@faint fern sigh
Hi
pls don't advertise in random channels
Bro how to make vote webhook ?
Plz help
we have an example here: https://github.com/top-gg/webhooks-v2-nodejs-example
hey Veld, is there new information on ratelimits being imposed on the v1 endpoints?
so i'm guessing 100/second for all routes?
currently working on rewriting the Python SDK π TODOs:
implement post commands for different librariesimplement webhooks- testing
- documentation
everything else is complete 
how about a bucket and just delay the response
for the Python SDK, i am using an existing ratelimit handler π
what routes are you rate limiting?
all routes
there is only one route /post for webhook?
const { type, data } = req.body;
const traceId = req.headers['x-topgg-trace'];
prettyRequest(traceId || 'No Trace ID', type, data);
res.status(200).send('Webhook received successfully');
});
if im not mistaken
okay so global ratelimiting would be working fine for those apis
you can also do it per user as you are taking the auth token

the Python SDK is almost done! all it needs now is testing and documentation! 
@dense barn ```js
const response2 = await fetch(https://top.gg/api/v1/projects/@me/votes?cursor=asdf, {
headers: {
Authorization: Bearer ${token},
'Content-Type': 'application/json'
}
})
gives 500
i hope you're not actually passing asdf
I am actually passing asdf
and yes I know what a valid cursor looks like
a valid cursor is a long hex string
I understand, just reminding you Veld π
@fair goblet could you add it to the backlog?
a bug is still a bug π
you don't have to fix it right now but still
but yeah I shouldn't have pinged you π sorry if I bothered you
I just went to bed lol but will do tomorrow
Thanks!
the Python SDK is done! all it needs now is testing and a proper documentation.
current coverage on the Python SDK as per pytest-cov: 100%
the Python SDK has been successfully tested with 100% coverage! 
hey @dense barn, do you mind if you dm me a public key for the lib dev access bot? i need it to write documentation examples for the Python SDK 
public key?
yeah
apparently some libraries like discord.http require it to be able to fetch global commands
you mean the bot token?
no, is there a mention of public key in the developer portal?
hikari requires this as well
yes
in the discord dev portal
btw i am doing this because i've been trying to document post_commands() usage for various libraries
do you guys think i should continue to do this? π
for starters the Rust SDK has special wrappers for Twilight and Serenity for this purpose
at this point i am 3 SDKs in and i'm reconsidering if i should continue doing this or not for the rest of the SDKs
@steady kite sorry for pinging you william but I need your thoughts on this
I would standardize it, so yes do it for the libraries that already have it that way if a user switches or whatever itβs already there if you known what I mean
I havenβt experienced exactly what those do, but if we can try to keep them as similar as possible I say to do that
Just my own thoughts, velds may differ ofc
ππ
actually on second thought I feel like since Veld wants for the SDKs to be updated to use v1 and be reviewed as quickly as possible, I assume this detail would be least of his concerns right now π I decided I'm not going through this for now, but I am going to open an issue for it for the SDKs that don't have it, so other contributors can help in the future!

the Python SDK's documentation is now complete! which means that the entire Python SDK is now finally complete and ready for review! 
sorry for the wait guys! π₯Ή
π₯³
I am gonna work on the rest of the SDKs in order based on their github's star count, sooo next stop is the Java SDK! 
everything in the Java SDK is pretty much complete! now it's time to add finishing touches π
alright! both are done!
fast
if you have the bot token, you could just fetch the application data and the public key is defined as verify_key (ref: https://docs.discord.com/developers/resources/application#get-current-application )
pretty sure you donβt even need a bot token for that :)
alright tests are halfway done! 
$mu
after 5 hours I've finally done webhooks testing for Dropwizard π two to go!
Eclipse Jetty has now been proven to work! only one left! π₯³
I'm going to continue tomorrow, please be patient!
alright, everything is pretty much done! all it needs now is the readme. the SDK is not entirely complete (i'll open github issues for them), but it's quite stable now.
README is now completee!!!
finally!
hey Veld, uhmmm I am about to open 9 more pull requests π
Awesome looking forward to it
new pull request open! https://github.com/Top-gg-Community/java-sdk/pull/24
this pull request's changes are auto-generated, so you can pretty much just approve it right away! 
alright good news! 5 (out of a previously 9) branches could be merged with their respective principal branch because their changes were not as huge as I thought
so now only 4 pull requests (including the one above) are open in the Java SDK's repository!
next stop: the .NET SDK
this one's gonna be super your alley Veld! hahahahaha! 
keep in mind that the progress on the remaining SDKs is going to be a bit slower as I have uni and other things to take care of too, sorry everyone!
if anyone wants to offer help, please be my guest! β€οΈ
both pull requests can now be reviewed!
I am now finally halfway through the SDKs! 
finally! i can go to sleep π hahahaha!

I am taking a break from the SDKs for a while, I really need to catch up with uni. Sorry everyone! π
now working on the Go SDK π making quick progress! 
alright, webhooks are done!
client tests are done!
webhook tests are done!
hey sir
next stop will be the C++ SDK! π it's not the most popular one but it's the one i'm the most familiar with out of the remaining 4 SDKs.
hey Veld! I've addressed your comments in #15. I've also reflected its changes in #16. 
i've added a comment for it 
thank you so much for your feedback! will work on them! 
will also reflect the changes in the other sdks if possible π
yea generally i would only fail (send a non 2XX status code) if authentication or authorization fails, all other errors are always on your side and should not make top.gg retry imo
adding a log is fair for parsing failing tho
alright! done! can you check it again? π
reflected the changes on the Python, Node.js, and Rust SDK
will work on updating the Java and .NET SDK later when I wake up! 
I really need more sleep lol
Added a small comment regarding io.ReadAll, nothing too big to change
thank you so much for your feedback kryptonn!!

will do!
thank YOU for your outstanding work on all of these SDKs <3
Also another comment on the other regarding linting
Now that work is being done on it, worth getting it to standards ^^
There were some other triggers, but mostly due to one PR fixing the triggers from the other PR, so not much relevant
Yeah you're the goat man!
huuuaaaaaaa that's quite the word π thank you so much everyone!
I am not perfect and I make so many mistakes!
β€οΈ
but I appreciate all of your support! It truly means the world to me! I'll always try to offer the best to all of you! π«Ά
yea your change looks good now, you could've inlined but since you reusing it doing a function is fine here
Everyone does! It's not about being perfect but being so consistent and dedicated that makes it amazing honestly
Reviewed the C# ones @drowsy thistle

@dense barn oh by the way, the long description for projects have length constraints, right?
Afaik, there's no upper limit but there's a minimum of 300
alright! thanks luke!
I think we did have to implement a maximum but I dont remember what it is
@dense barn do u remember
Its 65k
Integer limit
Yeah 16bit limit
Afaik at least
Thereβs a few bots with more
But i donβt think you can go beyond anymore
i see i see
hold on, I am updating my super long todo list π this is going to take some time! 
@dense barn i am not familiar with nuget but the nuget package has always been called DiscordBotsList.Api, no? wouldn't this require publishing a new nuget package?
the same also applies to the other SDKs, many of them still bear DBL in their package name. wouldn't this possibly require publishing an entirely new package?
Yeah it would need to be new package
noted noted!
updated the Go, Node, Rust, Python, .NET, and Java SDK for it
removed all references to DBL and updated the platform type enum!
oh btw @chilly yacht! i'm sorry but migrating from travis might be a bit outside of the pull request's scope π do you mind if i just get rid of travis for now? you can open a pull request for it later!
addressed
can you check them again?

Yeah that's better
I can add the workflow if needed, I use them everywhere already
sure!!
alright, all six SDKs are ready for review again! with the exceptions of the following low priority issues:
- The
warpwrapper for the Rust SDK does not include the payload JSON deserialization warning because to my knowledge it's not possible viawarp'sFilter#map()method. - The
actix-webwrapper for the Rust SDK does not include the 2 MiB request body limit because to my knowledge it's not possible to do this with a standaloneFromRequestimpl. - The Eclipse Jetty wrapper for the Java SDK does not include the 2 MiB request body limit because to my knowledge the library gives you the
Stringbody immediately from the@RequestBodyattribute, thus it's not possible to do this with a standaloneRestController.
while imo these issues are not worth tackling atm, they deserve their respective github issue in the future 
in case anybody wants to help contribute! β€οΈ
now writing a proper Javadoc documentation for the Java SDK! 
so far the main API has been documented. webhooks will take more time to document. please be patient.
okay I am starting to feel burned out. I am gonna take a break 
take your time null, don't burn yourself out
we appreicate your support 
Please do not rush yourself 
you are insane at this null
i'm still working on the lua sdk trying to get it to work as intended lol
alright! i am free now! now i can work on the sdks again! 
I am currently working on migrating the Node SDK from using undici to Node.js' native fetch() and from Jest to Node test. Please don't review it for now.
For the time being, can you guys review the Python, C#, Rust, and .NET SDK pull requests please?
you guys don't have to wait until I completed working on all SDKs before reviewing them! π
migration successful! the current coverage for the Node SDK is 100%! π
alright, the Node SDK is done and is ready for review! (for like the billionth time i know) 
new changes:
- Added timeout handling so that getBody() would not freeze forever if content-length does not match the actual body
- Squashed lots of webhook bugs
- Added mocked webhook unit tests
- Zero dependency vulnerabilities!
ping the link and I'll review it
i recommend you check out #95 first
because i feel like there could be conflicts between #95 and #92 π
sorry about that
if it's approved why not merge and fix the conflicts?
i can't.
but technically i could close #95, merge #95's changes into #92, then resolve its conflicts
even though that's kinda weird 
that's probably he better idea, just close 95 once you've merged the changes into 1 PR, makes more sense that way I believe, you can close 95 with a comment like "Approved changes merged into #95"

will do!
@pastel girder done! you can now check and review #92!
addressed your comment π it's now resolved!
does this include breaking changess?
yes
everything
the Node SDK is probably the only one to have everything inside one pull request 
ya little scarry
have the docs been updated to reflect these changes before it goes live
yeah in hindsight i feel like i should've split it
probably one of the biggest SDK
in terms of what?
usage
mhm π
the read me was at least updated so thats good
yes
including every code example
the readme will have the exact same format for the other sdks so i could enforce consistency
I love consistency
Looking at it, it looks good, as long as we have full test coverage over everything im happy with it going in
also added and tested webhook timeout handling to the Python SDK 
you guys could review that one as well 
I'm not sure the same could applied to the Java SDK's internal eclipse jetty, dropwizard, and spring boot wrappers as resources online said that "it's best to be configured at the server level." If anybody is experienced in these frameworks and you think otherwise, please don't hesitate to help me or create a pull request for it! π
also added and tested webhook timeout handling to the .NET SDK 
Looks fun to figure out O: I did some minor research and found out about two things that might work. DeferredResult instead of returning a synchronous response entity, the webhook controller method would return something like DeferredResult<ResponseEntity<?>> When a Spring controller returns this type the DispatcherServlet immediately releases the primary HTTP worker thread back to the embedded containers thread pool. Which would transition the request into an asynchronous state. And Servlet has an Asynchronous Processing API, plus CompletableFuture chaining from Java 8.
Idk if I got time I'll look at this one over the weekend
that's interesting, thank you so much for the insight capi!
β€οΈ
will look into it!
happy to help in any fashion
I love learning new things
Networking has been one of my top topics of research
awesomee!! β€οΈ
You can pass a specific timeout value to the deferredresult iirc.
Idk about dropwizard but I know it uses Jersey for web endpoints. You can pass a @Suspended AsyncResponse parameter to the webhook method. It also has a built in setTimeout method as well. If you're looking for ways to exactly timeout requests at 1000ms anyway

increased the default timeout to 5 seconds! 
added request timeout handling to the Rust SDK's actix-web, rocket, and axum wrappers!
I'm not sure how it's done for the warp wrapper π if anyone is knowledgable on this, please let me know!
added request timeout handling to the Go SDK! 
hai hai capi! this is what i came up with for the spring boot wrapper!
what do you think? π
(ignore the @Override errors π)
here's my code for it:
I see only one big issue
ForkJoinPool.commonPool() is really only optimized for cpu intensive tasks. Since webhook processing is almost always i/o bound using commonPool will exhaust the threads and bottleneck the server if a bunch of webhooks hit at once.
If devs can pass a custom ExecutorService into the constructor or if they don't provide one it can default to something better suited for i/o like Executors.newCachedThreadPool()
Other than that I can not see or uncover any sort of issues (from examining on my phone anyway
)
oh god!! π hahahaha
i will try! thank you so much for your feedback pal! β€οΈ
You're welcome! I don't have an environment in which I can test Java SDKs unfortunately, but I started off as a Java developer forever ago so I'm pretty familiar with how it works. I would definitely test anything I suggest or advise if possible
hahaha it's alright! π«Ά i can test them myself! i've written a pretty working unit test system for all webhooks π

any help is very appreciated, no matter how big or small β€οΈ
huuaaa thank you so much capii!!
β€οΈ
honestly you too with everything you make! you are equally as amazing!
I have my strengths and my weaknesses
my coding knowledge is self taught n all that. But that's why we work as a team 
me too!
i agree!
a person's strengths could cover another person's weaknesses and vice versa! π
If you end up looking at the pure jetty sdk too I wrote up a quick snippet of how you could adapt your logic to use an ExecutorService. jetty uses AsyncContext instead of DeferredResult to go async but u can basically just drop ur exact dispatchSync() logic right in.
import javax.servlet.AsyncContext;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.stream.Collectors;
public abstract class TopggWebhookServlet extends HttpServlet {
private byte[] secret;
private final ExecutorService executor;
public TopggWebhookServlet(final String secret, final ExecutorService executor) {
this.secret = secret.getBytes(java.nio.charset.StandardCharsets.UTF_8);
this.executor = executor != null ? executor : Executors.newCachedThreadPool();
}
public TopggWebhookServlet(final String secret) {
this(secret, null);
}
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) {
final AsyncContext async = req.startAsync();
try {
final String signatureHeader = req.getHeader("x-topgg-signature");
final String traceHeader = req.getHeader("x-topgg-trace");
final String body = req.getReader().lines().collect(Collectors.joining(System.lineSeparator()));
executor.submit(() -> {
try {
HttpServletResponse asyncResp = (HttpServletResponse) async.getResponse();
asyncResp.setStatus(HttpServletResponse.SC_OK);
} catch (Exception e) {
((HttpServletResponse) async.getResponse()).setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
} finally {
async.complete();
}
});
} catch (Exception e) {
resp.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
async.complete();
}
}
}
OH MY GOD I was literally writing this right now!! thank you so much though capi!
β€οΈ
this means a lot to me, it really does pal!

I'm glad I could help a little
I hopped to my PC for a second to type that our
Out*
you helped a little? i would say you helped a LOT π HOLY β€οΈ
Yeah I noticed the Java SDKs use synchronous blocking operations so switching to async is definitely a big win
that's a really commendable dedication β€οΈ i could never sufficiently express how thankful i am of your support and help!
you truly are the best at this capi!
π₯Ή
please don't say that. i appreciate your humbleness but you really are amazing β€οΈ
i am still learning Java right now π i am not sure how proficient i am at it but i am certainly trying my best!
If you did the dropwizard one already, a bit of a tip dropwizard uses jetty (and jersey for rest and routing) as it's embedded web server so you basically covered both with that one
But having a standalone jetty one for people running a lightweight sort of bot is always a nice option too
thank you for the tip capi!

will take note of it! π
@wispy carbon i am finally done with adding timeout handlers to the dropwizard and jetty wrappers! β€οΈ
here's what i wrote! π
dropwizard: https://pastes.dev/ORNinK2A6J
spring boot: https://pastes.dev/XzRr2qDYx9
jetty: https://pastes.dev/bet6FsZ3LT
i have finally documented every public class and method in the Java SDK!!! 
god damn this SDK is huge! hahahaha π
Just looked over all of these at lunch and dude, you absolutely nailed it. This is great. Consistency against all three sdks. Defaulting to a 100-thread fixed pool and a 5 second timeout across the board makes the whole sdk feel super cohesive no matter what framework someone might use. Using AsyncListeners and keeping the 2mb payload limit everywhere is a great security practice too.
Seriously amazing work modernizing all this. The java bot community is gonna love it.

huuuaaa thank you so much pal!! π₯Ή this means everything to me!
β€οΈ
Javadoc is now ready! 
i was unable to get external dependency hyperlinks and overview to work properly π but the entire SDK has now been documented! β€οΈ
i'll continue working on the C++ SDK in a few hours! 
my TODO right now: #general message
@drowsy thistle okay correct me if i am wrong isnt a bug
if (!signature.equals(HexFormat.of().formatHex(digest))) {
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
}```
it's too harsh
what do you mean?
The signature comparison uses a standard string equals method, which is not constant-time.
we mostly use java.security.MessageDigest.isEqual
also
assert signature != null && timestamp != null;```
it's most prob NullPointerException
default, assertions are disabled at runtime, and the assert statements are ignored by the JVM
If you're motivated to change every SDK to reflect that, it's a nice to have
I mainly didn't point it out because of the amount of time you've put in them and you're mainly maintaining them all alone, so adding other things on top of the implementations themselves and other is kinda wanting you to burn out
google Timing attack vulnerability
Realistically a timing attack on a signature is not the same as a timing attack on a password as they're not checked the same way as passwords
You'd need to compute the HMAC(secret, timestamp + "." + rawBody) all the time, good luck.
If you have a password being aaaaa, ab as input will return quicker, by milli/microseconds, than aab as input on non time constant checks
For simple string checking it's sort of easy/easier to take advantage of that
For signatures, have fun taking advantage of that... especially when a timestamp is in the content....
If you want, sure you can change them all as a "nice to have, never gonna happen", as said I didn't point it out as it would be intentionally burning you out
public TopggAPI(final OkHttpClient httpClient) {
gson =
new GsonBuilder()
.registerTypeAdapter(OffsetDateTime.class, new OffsetDateTimeConverter())
.registerTypeAdapter(PaginatedVotes.class, new PaginatedVotesConverter(this))
.create();
this.httpClient = httpClient;
}
i mean you are creating OkHttpClient every time it's called
use it like singleton
your work is amazing not gonna lie mate
but some bugs exists

it's alright pal, any feedback, harsh or not, is appreciated!
β€οΈ
it just means that you care. that's all that matters
will take note of this! π«Ά
so do i use the assert classes?
if (signature == null || timestamp == null) {
return ResponseEntity.status(HttpStatus.BAD_REQUEST).build();
}```
oh right! my bad!
my mind is soo clumsy!
Also one more recommendation donβt ignore exceptions throw it so user knows whatβs the error
i am doing this because i needed to make a mock for the tests π https://github.com/null8626/java-sdk/blob/tests/src/test/java/gg/top/api/TopggAPITests.java#L27-L33
I mean let user add there own client
So they can use it in there project
what do you mean?
Ahhh
Mb
Mb
.map(part -> part.split("=", 2))
This can be error if server receives invalid signature
.map(part -> {
String[] split = part.split("=", 2);
if (split.length < 2) return null;
return split;
})
.filter(Objects::nonNull)
uhmm temper? can you just open a review for it in my pull request?
what why?
Tom for sure
this too?
alright!!
i will. if i change one, i will change the rest to reflect it
even down to the most minute things like wordings in the documentation
if i were careless that would've been disrespectful π
anyway, I am about to go to bed. will address these issues in a few hours

Need more sleep than that

I wish I knew more about encryption and security things
same honestly, but it's alright! It does not make your contributions any less amazing β€οΈ
It's a learning experience for sure cx
@dense bronze @wispy carbon here's what i wrote! π«Ά what do you guys think? π
Itβs 5 in the morning will check in few hours
I made a silly lil sveltekit npm package for handling topgg webhooks. Not sure if I can share it
but I'd be happy to if it's okay
i just looked at it, it looks amazing! β€οΈ
there's nothing i love more than C++ generic errors π huaaa
π .
love you brain β€οΈ
hahahaha
nevermind, got it fixed! 
added and tested replay attack mitigation to the Node SDK's webhook integration! 
done the same thing to the Python, .NET, and Go SDK π
@wispy carbon @dense bronze added replay attack mitigration to the Java SDK's webhooks!
https://github.com/Top-gg-Community/java-sdk/pull/26/changes/df4e61e284332c202b6f737761e13b15c07ac75c
i totally forgot about this
sorry mate
it's alright pal!
lemme have a look
Good work null
yaaaayyy!!
β€οΈ
thank you so much!
i swear every time i open the rust sdk directory my storage space shrinks rapidly π huaaa

originally wanted to add replay attack mitigation, ended up rewriting how the rust sdk handles webhooks 
how on earth did i even write this
π
alright, done! now all it needs is an updated documentation! π
hey everyone! i will be taking a break in the next few days so expect development on the remaining sdks to slow down. I am so sorry for this... 
as much as I would love to get them out as soon as possible... my mind and motivation still have a finite capacity

in the meantime, please feel free to review the pull requests i've made for the node, python, c#, java, rust, and go sdks
please dont wait for me to finish all of them
those sdks are pretty popular and i'm sure developers would love to get access to them as soon as possible

as always, see pins for the pull request links!
Take as much time as you need dude, we really appreciate your hard work on the SDKs π
Hope you feel better after a nice long break honestly
Mera bot review kab hoga
@candid beacon we dont allow that here thank you
hay gais
@faint raft
yes?
Check private please
if it is related to a review dispute please discuss it over here -> #staff-tickets message
How do I change my botβs name on Top.gg?
please use #support channel

ban
Hello @drowsy thistle
is it yourself or someone else that mainly maintains the node sdk?
#support message this is why i ask, just wanted to flag it to someone to take a look at!
π«‘ will review the open PRs now
I read through and approved the ones I could 
the Node SDK's core maintainer is still Berry. I am just the one who made the pull request that adapts it to v1

my pull request removes these vulnerabilities iirc by using the native fetch() API instead of undici π
wait wait wait don't approve them just yet
i have not added the new v1 endpoints!
i appreciate it though β€οΈ
make a new PR for that please 
hahahaha fairs!
i plan to work on them in the coming weeks. please be patient everyone as my schedule is packed as hell 
@tiny shard can you merge https://github.com/Top-gg-Community/node-sdk/pull/92? thanks!
maybe i missed it somewhere and this was already discussed, but this seems to not cover the metrics/stats usecase completely? 90% of the usecase for the sdk is to go missing?
oh right above lol ok
makes sense, i just wouldnβt release it
merged
also, can we think about how we can do webhooks differently? maybe going back to the old listener system?
the callback makes sense when thereβs just one type but itβs kind of obnoxious once you get multiple types
@tiny shard @drowsy thistle y'all think it's possible to implement the last few endpoints soon so we can do a new release? 
don't worry veld! it certainly is possible! but i am crazy busy this weekend, soo maybe next week!
I've got some free time and added the last few endpoints 
https://github.com/Top-gg-Community/node-sdk/pull/96
reviewed!

thank you so much for your help pal! β€οΈ
Np! 
lgtm
whenever null checked I can merge it
approved!
hey veld, can you also review https://github.com/Top-gg-Community/python-sdk/pull/93? the pull request you've approved also depends on this one btw
PR build failed
fixed!
@dense barn added another pull request!
https://github.com/Top-gg-Community/node-sdk/pull/97
after an entire day i've also finished updating the Python SDK π₯Ή -- expect fresh pull requests from it soon!
the pull request for it is now openn!!! β€οΈ https://github.com/Top-gg-Community/python-sdk/pull/94
also updated https://github.com/Top-gg-Community/python-sdk/pull/92/, it can now be freely reviewed as well! π«Ά
will work on more SDKs when i have more time next week, sooo this is about the most i could do this week, i'm so sorry... 
Thanks!
I've reviewed and merged them
the docs one is a bit confusing to me since i dont really know the env
i understand! don't worry! you can review it visually by cloning the branch, and running these:
cd docs
pip install -r requirements.txt
make html
cd _build/html
index.html
the .rst files are self-explanatory, they are just a skeleton to automatically generate the documentation from the project's docstrings, analoguous to a markdown file with an autoclass specifier
the documentation's configuration is stored in conf.py
was working on updating the Rust SDK today but unfortunately I am crazy busy, quickly ran out of motivation, and burned out. I might continue working on it when I am available β€οΈ
I am so sorry everyone, I have a lot of things to balance π₯Ή
my mind has a limit and couldn't take on all things at once
@drowsy thistle no worries, take care of yourself β€οΈ
first
a
third message poggers UwU
rip the other one
no more old channel
sixth
im the second one
hehehe
im the first whitename to type in this channel, i should be proud
yes you should be :3
If you have unacknowledged code contributions to a public repo let me know so I can give you a transparent contributor role
π
aaaany repo?
pin it
as long as it's under top-gg
oh ok
oh lol
lol
I'll pin it later, just trying to gauge involvement
noice
a few people already have it. People seem to be interested in contributing so I wanna make it a nicer process
yet, at least

I'll open a few issues. We'll be making more things open source this year
cool
I love open source I wish our entire codebase could be OSS lol
but competitors
lol
I β€οΈ open source
can we contribute to beautiful code of https://github.com/top-gg/Luca 
I'd like to open source luca as well
cool
but I don't want to give bot reviewers even more shitty clones to deal with
I can make an open source Luca clone to publish x)
lol
aka undefiner
btw @high depot were you able to get it working after the downgrade?
rather than publishing the real one for people to make clones of
Undefiner is Luca but not Luca and has much more stuff
k
Yeah Iβm almost done. I need only to deal with objects I get from crowdin
Lemme make some tea rq
sure
No memes 
I love open sauce
we're working on https://github.com/top-gg/crowdin-discord btw

hiiiii oliy!!
At this point Iβd say that that crowdin sucks
wait top.gg open source? I thought this was a place where people could share their open source works
no top.gg open source
404 moment
yes it's not ready yet
u could share ur open source stuff in #development though
cuz that would be dev related
slide left
too lazy x)
kek
so anyways
its already perfect
@marble parcel im the first person to send a message lol
and im the first whitename
I remember specifically saying pls no memes in this channel 5 seconds ago
?
Lol
luca is open source
no
it totally is
don't u see this beautiful code
its a work of art
yes
@hollow jacinth 
what the-
thanks for curing my depression
just wondering, what can "we". The dev community do with the open source?
to contribute
if ya understand what I mean
We're doing our best to provide some sort of OSS culture from our company side
we'll also have more stuff to contribute to later
Considering we have a dev-heavy community, It would be greatly appreciated to start projects together to better the entire ecosystem
And in return we open source our internal projects too
interesting
I'll be open sourcing the internationalization tooling I wrote for top.gg once I move it out to its own repo 
could be useful seeing as there's like close to no tooling for translations
finally i can start my bottum.com for cool sa as bots by git cloning top.gg website
Open source Luca
yardΔ±m my open source softwareβ’οΈ
considering how excited people were about #site-translators I think we could do something similar with code if people feel their contributions are going to be useful which I think is what gets people excited about doing translations
yea
though tbh, there should be active maintainers of libraries though, for example the java library still has open prs/issues since 2019 and nik, presumably the maintainer, doesn't do anything anymore
yeah we should make sure those repos are alive at some point
||#ApplicationsWaitingTeam
||
especially once apiv1 is alive
yup
For a hot second I thought we could finally help the team fix FE and BE issues so people wouldn't have a broken top.gg site for months π¦
i didnt contributed to a big oss project before this will be my first please be kind 
i am disappoint
I would love to open source everything sadly that decision is not up to me lol
devs always want to open source, management never wants to.
atVeld plz OSS everything kthxbye
OSS topgg backend and nextjs frontend so we can fix shit π
I don't care about bots
:0
Of course it's safe. In the end it's safer because you can get community-provided security fixes.
We're not open sourcing the product 

One message removed from a suspended account.
read pins

One message removed from a suspended account.
One message removed from a suspended account.
ΓΆ
One message removed from a suspended account.
One message removed from a suspended account.
dum me
Δ
shit yeah, when do I get demoted am bad mod
this became general-3
One message removed from a suspended account.
One message removed from a suspended account.
One message removed from a suspended account.
One message removed from a suspended account.
all of it
Open-Source Software
One message removed from a suspended account.
One message removed from a suspended account.
Huh
if you wanna discuss prs, issues, and new features etc
This is basically the github repo discussions section but in Discord
yes
One message removed from a suspended account.
One message removed from a suspended account.
had some discussions about the docs and the new repo we were working on and nowhere to talk about them
what about development channel
development is any project
ic
One message removed from a suspended account.
One message removed from a suspended account.
One message removed from a suspended account.
no
One message removed from a suspended account.
no
One message removed from a suspended account.
That repo is art
Yes
But could be better if the actual Luca's source is OSS
One message removed from a suspended account.
One message removed from a suspended account.
So lib dev was merged into this channel along with other OSS I assume
One message removed from a suspended account.
Sadly luca will not truly be open source cuz they wanna prevent luca clones
Oui
One message removed from a suspended account.
One message removed from a suspended account.
lol
One message removed from a suspended account.
It's something we wanna test out
One message removed from a suspended account.
so less work for bot reviewers
One message removed from a suspended account.
Sounds like an interesting ride
One message removed from a suspended account.
but guys,
Wait, how do u get that role even wut
oh sorry I forgot about you @dusky shale
Xet moment, aight moving to general for random convos
yeah preferably anything that wouldn't piss off maintainers in hacktoberfest
lmao ok
fixing documentation
fixing typos
fixing bugs
adding new features
adding a comma to README.md
bumping a year in LICENSE
you can use your common sense to fill in the gaps for everything else
owo whats this
does this mean that you're willing to open source the frontend after the beta is concluded?

i will happily contribute even more than i already am if that's the case
there are numerous suggestions i would love to pitch in as PRs
because xetera is stinky
Submission guidelines
fixing documentation
fixing typos
fixing bugs
adding new features
adding a comma to README.md
bumping a year in LICENSE
@high depot feel free to push your changes when you're done btw
why bump the year though
already done
oh? I didn't see it
pushing them rn
ah ok
just making sure that i deleted all links and tokens
haha
fin

let stringChangeAmount; if (eventsStructured.hasOwnProperty('string.updated')) {stringChangeAmount = JSON.stringify(eventsStructured["string.updated"].length)} else {stringChangeAmount = 0}
should probably use a proper discord request in order to handle ratelimits no?
probably isnt necessary but might be a good idea
also a bunch of weird formatting things going on there but
I'll polish the rest of it up, thanks 
let stringAddAmount; if (eventsStructured.hasOwnProperty('string.added')) {stringAddAmount = JSON.stringify(eventsStructured["string.added"].length)} else {stringAddAmount = 0}``` :bruh-4:
would make more sense to use ternary operators but it works Β―_(γ)_/Β―
ternary is not really readable for people other than you tho
not if you use newlines?
I can do it but I was too bored to think about that 
but for this i would just do
let stringAddAmount = eventsStructured.hasOwnProperty('string.added')
? JSON.stringify(eventsStructured["string.added"].length)
: 0```
const stringAddAmount = 'string.added' in eventsStructured ? JSON.stringify(eventsStructured['string.added'].length) : 0;```
a ? b : c β
(a) ? (b) : (c) makes it more readable if in one line
in terms of code maintainability, it's fine, this is just formatting.


load balancer slices




