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

Apply default outputDir indexing logic to user-provided output dir #461

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

valeryvpetrov-dev
Copy link

Problem

If I pass --output-dir argument with my custom value then I get report in pointed dir. But when I run profiler for the second time it overwrites previous report.

Solution

If I do not pass --output-dir argument profiler then creates it by itself in profile-out-* dirs. So each run report is placed in separate dir. So why not do the same with custom output dir?

Results

Signed-off-by: Valery Petrov <valeryvpetrov.dev@gmail.com>
Signed-off-by: Valery Petrov <valeryvpetrov.dev@gmail.com>
@gabrielfeo
Copy link

Disclaimer: I am not a project member

IMHO, profiler should keep writing results directly to the --output-dir, without creating subdirs on its own. This way, output files have a predictable location, which is better for automation, e.g. running profiler and uploading the outputdir/benchmark.csv somewhere.

However, I've also overwritten profiler results by mistake as you described. Perhaps an alternative solution would be making profiler fail if --output-dir is not empty.

@valeryvpetrov-dev
Copy link
Author

@gabrielfeo good point about automation. When I used profiler for the first time I expect to have unified behavior. But it differs. I see three solutions:

  • Make --ouput-dir generation of indexed dirs in case of custom value
  • Make --output-dir generation to the same dir between runs for default value
  • Update documentation for the case of custom value

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.

None yet

2 participants