Skip to content

Safely parse env file#195

Open
shopeonarope wants to merge 1 commit intoMcCloudS:mainfrom
shopeonarope:main
Open

Safely parse env file#195
shopeonarope wants to merge 1 commit intoMcCloudS:mainfrom
shopeonarope:main

Conversation

@shopeonarope
Copy link
Copy Markdown

Protect against ValueError exception when a line doesn't have an = like an empty line or a comment

Protect against ValueError exception when a line doesn't have an = like an empty line or a comment
@muisje
Copy link
Copy Markdown
Contributor

muisje commented Feb 28, 2025

Seems like this would accept "wrong" configurations where comments are not started with e.g. #

I implemented this solution:

https://github.com/muisje/subgen/blob/LanguageCode-improvements/load_env_variables.py

@muisje
Copy link
Copy Markdown
Contributor

muisje commented Feb 28, 2025

And here's an example file that shows what I think should be parsed (and does in my branch)

https://github.com/muisje/subgen/blob/LanguageCode-improvements/subgen.env.example

@shopeonarope
Copy link
Copy Markdown
Author

That looks nice. You should open a PR to get it integrated.

subgen.py Outdated
webhookport = int(os.getenv('WEBHOOKPORT', 9000))
word_level_highlight = convert_to_bool(os.getenv('WORD_LEVEL_HIGHLIGHT', False))
word_level_highlight = convert_to_bool(os.getenv('WORD_LEVEL_HIGHLIGHT', True))
segment_level = convert_to_bool(os.getenv('SEGMENT_LEVEL', False))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the use case here? SEGMENT_LEVEL is True by default.

subgen.py Outdated
text=appended_text,
words=[], # Empty list for words
id=lastSegment.id + 1
words=[
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beyond being done differently, what does is change?

subgen.py Outdated
}
)
if output == 'srt':
return Response(
Copy link
Copy Markdown
Owner

@McCloudS McCloudS Mar 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reluctant to implement response instead of StreamingResponse. I see no downside to StreamingResponse whereas Response will start to have performance issues on returned files exceeding 10-50mb.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StreamingResponse was really slow delivering the response, changing to Response made it much more performant. It was just a quick solution for me, don't read much into it.

subgen.py Outdated
'Source': 'Transcribed using stable-ts from Subgen!',
}
)
elif output == 'json':
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not opposed to implementing, what's the usecase?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bleh, sorry for the mess up. I pushed changes to my main unrelated to this PR. I've reverted to the original changeset. That'll teach me to use a branch on my fork. But to answer the question, I am doing some post-processing using the word timings so having the raw result JSON is handy for that.

subgen.py Outdated
namesublang = os.getenv('NAMESUBLANG', '')
webhookport = int(os.getenv('WEBHOOKPORT', 9000))
word_level_highlight = convert_to_bool(os.getenv('WORD_LEVEL_HIGHLIGHT', False))
word_level_highlight = convert_to_bool(os.getenv('WORD_LEVEL_HIGHLIGHT', True))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to stay default False, otherwise lots of people will complain when their subs start highlighting words as they are displayed.

subgen.py Outdated
if not force_language:
force_language = LanguageCode.from_string(result.language)
result.to_srt_vtt(name_subtitle(file_path, force_language), word_level=word_level_highlight)
result.to_srt_vtt(name_subtitle(file_path, force_language), word_level=word_level_highlight, segment_level=segment_level)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, what's the use case for changing segment_level to false?

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.

3 participants