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

Fix #276: Use regex to parse config file #284

Merged
merged 1 commit into from
Jun 24, 2023

Conversation

boschmitt
Copy link
Collaborator

Description

Slightly more robust solution to parse target config scripts. We must re-address this when we refactor the driver.

@boschmitt boschmitt force-pushed the fix_bug_276 branch 2 times, most recently from a5e97ed to d7d9f98 Compare June 23, 2023 07:04
@schweitzpgi
Copy link
Collaborator

Won't this match config lines that are commented out?

@schweitzpgi
Copy link
Collaborator

It could be more robust to have the options passed in as command line arguments or environment variables rather than trying to reparse the configuration file.

@boschmitt
Copy link
Collaborator Author

boschmitt commented Jun 23, 2023

Won't this match config lines that are commented out?

It will indeed. I can adjust the regex. (Edit: but then someone could come up with modifying this variable in some if statement, or use a +=. Unless we implement something as suggested bellow, there will most likely be loose ends)

It could be more robust to have the options passed in as command line arguments or environment variables rather than trying to reparse the configuration file.

Actually this is what I thought that was happening from the beginning. Since I'm unsure if other parts of the runtime are relying on parsing this file, I went to with trying to fix it by changing overall behavior as little as possible.

@boschmitt boschmitt enabled auto-merge (squash) June 24, 2023 15:26
@boschmitt boschmitt merged commit 365b264 into NVIDIA:main Jun 24, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 24, 2023
@bettinaheim bettinaheim added the maintenance Work items to update and improve the code base label Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintenance Work items to update and improve the code base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants