-
Notifications
You must be signed in to change notification settings - Fork 224
update YoutubeLoader #276
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
base: main
Are you sure you want to change the base?
update YoutubeLoader #276
Conversation
|
||
try: | ||
transcript_list = YouTubeTranscriptApi.list_transcripts(self.video_id) | ||
transcript_list = YouTubeTranscriptApi.list(self.video_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change from YouTubeTranscriptApi.list_transcripts(self.video_id)
to YouTubeTranscriptApi.list(self.video_id)
may introduce breaking changes if this YouTube document loader is part of the public API. Modifying the underlying method call could break downstream consumers relying on specific behavior or error handling patterns of the list_transcripts
method. Before implementing this change, verify that: 1) The list
method provides identical functionality and error handling as list_transcripts
, 2) The change maintains backward compatibility, and 3) This modification affects only internal implementation details rather than public interface dependencies.
transcript_list = YouTubeTranscriptApi.list(self.video_id) | |
transcript_list = YouTubeTranscriptApi.list_transcripts(self.video_id) |
Spotted by Diamond (based on custom rule: Code quality)
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per the latest version of document, to list all transcripts which are available, it should use list()
instead of list_transcripts
|
||
try: | ||
transcript_list = YouTubeTranscriptApi.list_transcripts(self.video_id) | ||
transcript_list = YouTubeTranscriptApi().list(self.video_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change introduces a breaking API modification. The code transitions from calling the static method YouTubeTranscriptApi.list_transcripts()
to calling an instance method YouTubeTranscriptApi().list()
. This alteration breaks backward compatibility by:
- Requiring instantiation of the
YouTubeTranscriptApi
class instead of static method access - Changing the method name from
list_transcripts
tolist
This modification could impact downstream code dependencies and any direct callers of this method. To maintain compatibility, either preserve the original static method call or document this as a breaking change with appropriate version increment.
transcript_list = YouTubeTranscriptApi().list(self.video_id) | |
transcript_list = YouTubeTranscriptApi.list_transcripts(self.video_id) |
Spotted by Diamond (based on custom rule: Code quality)
Is this helpful? React 👍 or 👎 to let us know.
update list_transcripts to list