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

Get Stream Key and URL through Client #17

Closed
wants to merge 0 commits into from
Closed

Conversation

gub-7
Copy link

@gub-7 gub-7 commented Nov 14, 2024

Modified http and client code to gain access to stream url and stream key

@cibere
Copy link
Owner

cibere commented Nov 14, 2024

Has this been tested? i can see a potential error with this

@gub-7
Copy link
Author

gub-7 commented Nov 15, 2024

Has this been tested? i can see a potential error with this

tested pretty thoroughly, it's working fine for me. I'll fix these failing checks in a couple days. If you've got any suggestions on how to make this change more faithfully to how you've constructed the project please let me know or push a commit to this branch. I intend on using this client pretty heavily in a new project of mine.

@cibere
Copy link
Owner

cibere commented Nov 15, 2024

Has this been tested? i can see a potential error with this

tested pretty thoroughly, it's working fine for me. I'll fix these failing checks in a couple days. If you've got any suggestions on how to make this change more faithfully to how you've constructed the project please let me know or push a commit to this branch. I intend on using this client pretty heavily in a new project of mine.

In the HTTP client you return with a payload of the StreamURLKeyPayload type, however in the Client endpoint you use the HTTP endpoint, return the exact result, but type it as a str and not the payload/dictionary that is supposedly returned.
So either you typed the payload wrong, or you typed the client wrong and are getting the key yourself

@gub-7
Copy link
Author

gub-7 commented Nov 18, 2024

Has this been tested? i can see a potential error with this

tested pretty thoroughly, it's working fine for me. I'll fix these failing checks in a couple days. If you've got any suggestions on how to make this change more faithfully to how you've constructed the project please let me know or push a commit to this branch. I intend on using this client pretty heavily in a new project of mine.

In the HTTP client you return with a payload of the StreamURLKeyPayload type, however in the Client endpoint you use the HTTP endpoint, return the exact result, but type it as a str and not the payload/dictionary that is supposedly returned. So either you typed the payload wrong, or you typed the client wrong and are getting the key yourself

Thanks for pointing that out. Still works as intended because I think python is just inferring that since StreamURLKeyPayload only contains 1 member and it's a string I intend to return that string, but I'll make sure to clean that up.

@cibere
Copy link
Owner

cibere commented Nov 18, 2024

Has this been tested? i can see a potential error with this

tested pretty thoroughly, it's working fine for me. I'll fix these failing checks in a couple days. If you've got any suggestions on how to make this change more faithfully to how you've constructed the project please let me know or push a commit to this branch. I intend on using this client pretty heavily in a new project of mine.

In the HTTP client you return with a payload of the StreamURLKeyPayload type, however in the Client endpoint you use the HTTP endpoint, return the exact result, but type it as a str and not the payload/dictionary that is supposedly returned. So either you typed the payload wrong, or you typed the client wrong and are getting the key yourself

Thanks for pointing that out. Still works as intended because I think python is just inferring that since StreamURLKeyPayload only contains 1 member and it's a string I intend to return that string, but I'll make sure to clean that up.

I noticed that you fixed this, but could you fix it a different way? Exposing raw payloads to the user isn't something that I would like, so perhaps having the Client.fetch_stream_url_and_key method get the ingest_url key value and returning the url, while keeping the original typing that you had

@gub-7
Copy link
Author

gub-7 commented Nov 19, 2024

Exposing raw payloads to the user isn't something that I would like, so perhaps having the Client.fetch_stream_url_and_key method get the ingest_url key value and returning the url, while keeping the original typing that you had

sure thing.

@cibere
Copy link
Owner

cibere commented Nov 23, 2024

Thanks for this hard work. There are still some things that I would like changed, but I can always fix them myself. Just let me know if you want me to, or if you want to fix them yourself.

@gub-7
Copy link
Author

gub-7 commented Nov 23, 2024

Thanks for this hard work. There are still some things that I would like changed, but I can always fix them myself. Just let me know if you want me to, or if you want to fix them yourself.

No worries man, i'm working on some project that needs the functionality i'm adding, so happy to add it here. Let me know what those things are you would like fixed, i'll fix them. Accidentally muddy'd this PR by committing some changes to the main branch of my fork for a different feature, i'll be sure to separate them out into separate PRs when i'm finished. in the meantime you're welcome to provide any relevant feedback

@gub-7
Copy link
Author

gub-7 commented Nov 24, 2024

Resubmitting as 4 separate PRs.

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

Successfully merging this pull request may close these issues.

2 participants