Skip to content

Conversation

brandonbiggs
Copy link

I wanted to try this on a single file before I added it to the others.

@brandonbiggs
Copy link
Author

I added the signoff this time in my commit but the error is still there? Not sure why that's happening?

@mraineri
Copy link
Contributor

I added the signoff this time in my commit but the error is still there? Not sure why that's happening?

Very strange, I don't know why... Did you use the exact commands the DCO checker recommended, or did you sign it off another way?

@mraineri
Copy link
Contributor

As a general comment, I had been thinking about doing something just like this for resolving issue #174. One thing that I was considering was making this handled in a common manner so that we don't have to copy/paste the same code in the different scripts. We started using redfish_utilities/misc.py to contain other common logic.

@brandonbiggs
Copy link
Author

I added the signoff this time in my commit but the error is still there? Not sure why that's happening?

Very strange, I don't know why... Did you use the exact commands the DCO checker recommended, or did you sign it off another way?

I did use the --signoff and I can see the signoff addition in my commit. I'll take a look at the DCO check to see if I'm still missing something.

Brandon Biggs and others added 3 commits March 11, 2025 21:36
…logger script that allows for more options when logging. Updated rf_diagnostic_data with the new proposed argument and logger scheme

Signed-off-by: Brandon Biggs <[email protected]>
@brandonbiggs
Copy link
Author

The DCO issue is my fault. It's expecting the hostname of the system I'm on rather than my actual email. Not sure how to fix that, but my signoffs do appear to be in the commits.

@brandonbiggs
Copy link
Author

@mraineri I created a new file for arguments and one for logging. Tried to put common arguments into the new arguments.py. Thoughts on this style?

def create_parent_parser(description: str = "", auth: bool = False, rhost: bool = False):
parent_parser = argparse.ArgumentParser(description=description, add_help=False)

parent_parser.add_argument(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we maintain parity with the existing --debug argument for the time being? I have a lot of folks relying on this usage now to debug everything.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I'm back to this. Sorry it took me so long. I believe I added back the existing --debug functionality in diagnostic_data. I may have misunderstood you though.

@mraineri
Copy link
Contributor

Other than those few comments, this is looking really nice!

brandonbiggs and others added 15 commits May 10, 2025 21:21
…logger script that allows for more options when logging. Updated rf_diagnostic_data with the new proposed argument and logger scheme

Signed-off-by: Brandon Biggs <[email protected]>
Signed-off-by: Mike Raineri <[email protected]>
Signed-off-by: Brandon Biggs <[email protected]>
Signed-off-by: GitHub Release Workflow <>
Signed-off-by: Brandon Biggs <[email protected]>
…g a manager's time

Signed-off-by: Mike Raineri <[email protected]>
Signed-off-by: Brandon Biggs <[email protected]>
Signed-off-by: GitHub Release Workflow <>
Signed-off-by: Brandon Biggs <[email protected]>
… that is used everywhere else

Signed-off-by: Brandon Biggs <[email protected]>
Signed-off-by: Brandon Biggs <[email protected]>
Signed-off-by: Brandon Biggs <[email protected]>
@brandonbiggs
Copy link
Author

Okay, sorry for all the extra commits on this, tried to get it to fix the checks. I think I was able to resolve your concerns, please let me know if not. I tried to maintain parity with the current debugging. I think I'd like to clean up the session token stuff a bit, but I'd like to close out any lingering issues with this PR since it's a bit older. Let me know what you think.

raise ValueError(f"Invalid debug level: {level}")


def setup_logger(
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the --debug option isn't used when setting up the debug output. Can we add it in so that if specified, it logs to a file (and is set to "loggingDEBUG")?

@mraineri
Copy link
Contributor

@brandonbiggs sorry I missed the fact you were pushing updates.... There's one last nit with the debug options (mainly ensuring that if a user specifies --debug, they still get the trace file with the fullest logging). If we get that (plus the DCO sign-offs) this should be good to go.

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