Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type mismatches with ESI and friction with other crates #38

Closed
jhelwig opened this issue Jan 30, 2024 · 6 comments
Closed

Type mismatches with ESI and friction with other crates #38

jhelwig opened this issue Jan 30, 2024 · 6 comments

Comments

@jhelwig
Copy link
Contributor

jhelwig commented Jan 30, 2024

One thing I've been noticing as I've been going through various endpoints is that rfesi uses u32/u64 in a number of places where the ESI docs say it uses an integer32/integer64 (most commonly with IDs). There's also that rfesi uses u128 for the access token expiration (millisecond timestamp), but chrono uses i64 (DateTime timestamp_millis). It is very unlikely that CCP will give an access token that expires before 1970, but it does make it a bit awkward to work with the expiration.

While it's very unlikely that CCP will be returning negative numbers in a lot of these places, they do only document them as "integer", instead of as "unsigned integer". I've been hitting this with IDs, because I'm using PostgreSQL, and it doesn't support unsigned integers without a 3rd party extension (mostly because they're not part of the SQL standard). Because PostgreSQL & the SQL standard don't support unsigned ints, sqlx also doesn't support them.

Mostly I wanted to bring these up as awkward points of integration with other things, and see how you've been handling them, and what your thoughts where on the reasons behind the specific type choices.

@Celeo
Copy link
Owner

Celeo commented Jan 30, 2024

Hi!

I originally wrote integer types as unsigned, since I wasn't observing any negative values, and wanted to be somewhat restrictive with the types. Why? Don't remember. In more recent code, I tried to match the types given by CCP, but I didn't go back and fix mismatches from before, mostly because no one mentioned it being a problem. I'm happy to, though, and agree that it'd be better overall to be as accurate as possible.

@Celeo
Copy link
Owner

Celeo commented Jan 30, 2024

dacf4f7

@Celeo
Copy link
Owner

Celeo commented Jan 30, 2024

de5a845

@Celeo
Copy link
Owner

Celeo commented Jan 30, 2024

Released in 0.41.0 and 0.41.1.

@jhelwig
Copy link
Contributor Author

jhelwig commented Jan 30, 2024

Thanks!

rfesi has been a big help in bootstrapping the app I’ve been working on. Thanks for putting it out there.

@Celeo
Copy link
Owner

Celeo commented Feb 6, 2024

Let me know if there's anything missing from this effort, and I'll happily address. Thanks!

@Celeo Celeo closed this as completed Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants