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

log. _prefix_from_stack() needs a defined behaviour. #185

Open
terjekv opened this issue Dec 6, 2023 · 0 comments
Open

log. _prefix_from_stack() needs a defined behaviour. #185

terjekv opened this issue Dec 6, 2023 · 0 comments
Labels
bug Something isn't working

Comments

@terjekv
Copy link
Collaborator

terjekv commented Dec 6, 2023

def _prefix_from_stack() -> str:
is supposed (?) to help debug where a warning or error happens in the code, but due to its filtering it rarely actually reports anything useful.

The question is if this stack parsing should be exposed to the users or if it should be logged or available as debug information. Ideally we probably want messages back to the user to make sense for the user if it is something they can do differently. Right now, even warnings such as host not found tries to provide information from the stack and that seems counterproductive to say the least.

@terjekv terjekv added the bug Something isn't working label Dec 6, 2023
terjekv added a commit to terjekv/mreg-cli that referenced this issue Dec 6, 2023
## Changes

  - Formats command events into properly related JSON.
  - Properly record status responses (ok, warning, errors).
  - Record API requests as a chain related to each command.
  - Record the raw command issued, the actual command executed, and the command filter applied (if any).
  - Moves comment handling to OutputManager.
  - Consolidates stop_recording and save_recording.
  - Renamed some variables and methods for consistency.

## Issues

  - We want timestamps in the logs, but they will differ for CI runs and thus fail. There needs to be a configuration option to disable logging timestamps...
  - The stack details for errors and warnings is bugged. See unioslo#185
  - We buffer the entire recording blob in memory until we stop recording (or we exit). This is to ensure valid JSON for the output, as writing (valid) JSON streams to a file is non-trivial. A solution is to write after each command, reload the file and append the data, then write again. This may or may not be a good idea, and it would look something like:

```python
            existing_data = []

            # Read the existing data from the file
            try:
                with open(self._filename, "r") as rec_file:
                    existing_data = json.load(rec_file)
            except (FileNotFoundError, json.JSONDecodeError):
                pass

            existing_data.append(recording_entry)

            # Write the updated data back to the file
            with open(self._filename, "w") as rec_file:
                json.dump(existing_data, rec_file, indent=2)
```

## Failing tests

As this completely redefines the recording output, tests fail miserably.
terjekv added a commit to terjekv/mreg-cli that referenced this issue Dec 6, 2023
## Changes

  - Formats command events into properly related JSON.
  - Properly record status responses (ok, warning, errors).
  - Record API requests as a chain related to each command.
  - Record the raw command issued, the actual command executed, and the command filter applied (if any).
  - Moves comment handling to OutputManager.
  - Consolidates stop_recording and save_recording.
  - Renamed some variables and methods for consistency.

## Issues

  - We want timestamps in the logs, but they will differ for CI runs and thus fail. There needs to be a configuration option to disable logging timestamps...
  - The stack details for errors and warnings is bugged. See unioslo#185
  - We buffer the entire recording blob in memory until we stop recording (or we exit). This is to ensure valid JSON for the output, as writing (valid) JSON streams to a file is non-trivial. A solution is to write after each command, reload the file and append the data, then write again. This may or may not be a good idea, and it would look something like:

```python
            existing_data = []

            # Read the existing data from the file
            try:
                with open(self._filename, "r") as rec_file:
                    existing_data = json.load(rec_file)
            except (FileNotFoundError, json.JSONDecodeError):
                pass

            existing_data.append(recording_entry)

            # Write the updated data back to the file
            with open(self._filename, "w") as rec_file:
                json.dump(existing_data, rec_file, indent=2)
```

## Failing tests

As this completely redefines the recording output, tests fail miserably.
terjekv added a commit to terjekv/mreg-cli that referenced this issue Dec 11, 2023
## Changes

  - Formats command events into properly related JSON.
  - Properly record status responses (ok, warning, errors).
  - Record API requests as a chain related to each command.
  - Record the raw command issued, the actual command executed, and the command filter applied (if any).
  - Moves comment handling to OutputManager.
  - Consolidates stop_recording and save_recording.
  - Renamed some variables and methods for consistency.

## Issues

  - We want timestamps in the logs, but they will differ for CI runs and thus fail. There needs to be a configuration option to disable logging timestamps...
  - The stack details for errors and warnings is bugged. See unioslo#185
  - We buffer the entire recording blob in memory until we stop recording (or we exit). This is to ensure valid JSON for the output, as writing (valid) JSON streams to a file is non-trivial. A solution is to write after each command, reload the file and append the data, then write again. This may or may not be a good idea, and it would look something like:

```python
            existing_data = []

            # Read the existing data from the file
            try:
                with open(self._filename, "r") as rec_file:
                    existing_data = json.load(rec_file)
            except (FileNotFoundError, json.JSONDecodeError):
                pass

            existing_data.append(recording_entry)

            # Write the updated data back to the file
            with open(self._filename, "w") as rec_file:
                json.dump(existing_data, rec_file, indent=2)
```

## Failing tests

As this completely redefines the recording output, tests fail miserably.
terjekv added a commit to terjekv/mreg-cli that referenced this issue Dec 12, 2023
## Changes

  - Formats command events into properly related JSON.
  - Properly record status responses (ok, warning, errors).
  - Record API requests as a chain related to each command.
  - Record the raw command issued, the actual command executed, and the command filter applied (if any).
  - Moves comment handling to OutputManager.
  - Consolidates stop_recording and save_recording.
  - Renamed some variables and methods for consistency.

## Issues

  - We want timestamps in the logs, but they will differ for CI runs and thus fail. There needs to be a configuration option to disable logging timestamps...
  - The stack details for errors and warnings is bugged. See unioslo#185
  - We buffer the entire recording blob in memory until we stop recording (or we exit). This is to ensure valid JSON for the output, as writing (valid) JSON streams to a file is non-trivial. A solution is to write after each command, reload the file and append the data, then write again. This may or may not be a good idea, and it would look something like:

```python
            existing_data = []

            # Read the existing data from the file
            try:
                with open(self._filename, "r") as rec_file:
                    existing_data = json.load(rec_file)
            except (FileNotFoundError, json.JSONDecodeError):
                pass

            existing_data.append(recording_entry)

            # Write the updated data back to the file
            with open(self._filename, "w") as rec_file:
                json.dump(existing_data, rec_file, indent=2)
```

## Failing tests

As this completely redefines the recording output, tests fail miserably.
terjekv added a commit to terjekv/mreg-cli that referenced this issue Dec 13, 2023
  - Formats command events into properly related JSON.
  - Properly record status responses (ok, warning, errors).
  - Record API requests as a chain related to each command.
  - Record the raw command issued, the actual command executed, and the command filter applied (if any).
  - Moves comment handling to OutputManager.
  - Consolidates stop_recording and save_recording.
  - Renamed some variables and methods for consistency.

  - We want timestamps in the logs, but they will differ for CI runs and thus fail. There needs to be a configuration option to disable logging timestamps...
  - The stack details for errors and warnings is bugged. See unioslo#185
  - We buffer the entire recording blob in memory until we stop recording (or we exit). This is to ensure valid JSON for the output, as writing (valid) JSON streams to a file is non-trivial. A solution is to write after each command, reload the file and append the data, then write again. This may or may not be a good idea, and it would look something like:

```python
            existing_data = []

            # Read the existing data from the file
            try:
                with open(self._filename, "r") as rec_file:
                    existing_data = json.load(rec_file)
            except (FileNotFoundError, json.JSONDecodeError):
                pass

            existing_data.append(recording_entry)

            # Write the updated data back to the file
            with open(self._filename, "w") as rec_file:
                json.dump(existing_data, rec_file, indent=2)
```

As this completely redefines the recording output, tests fail miserably.
terjekv added a commit that referenced this issue Dec 18, 2023
* Reformat and redesign the recording output.

  - Formats command events into properly related JSON.
  - Properly record status responses (ok, warning, errors).
  - Record API requests as a chain related to each command.
  - Record the raw command issued, the actual command executed, and the command filter applied (if any).
  - Moves comment handling to OutputManager.
  - Consolidates stop_recording and save_recording.
  - Renamed some variables and methods for consistency.
  - Recording-related methods now all start with recording_ as a prefix.
  - Added a command line option to enable or disable timing data from the recorded data.
  - In general, callers adding details to the recorded data shouldn't test if recording is active. OutputManager handles that detail, the rest of the code simply states that it wishes to make something available for recording.

* Issues

  - The stack details for errors and warnings is bugged. See #185
  - We buffer the entire recording blob in memory until we stop recording (or we exit). This is to ensure valid JSON for the output, as writing (valid) JSON streams to a file is non-trivial. A solution is to write after each command, reload the file and append the data, then write again. This may or may not be a good idea, and it would look something like:

```python
            existing_data = []

            # Read the existing data from the file
            try:
                with open(self._filename, "r") as rec_file:
                    existing_data = json.load(rec_file)
            except (FileNotFoundError, json.JSONDecodeError):
                pass

            existing_data.append(recording_entry)

            # Write the updated data back to the file
            with open(self._filename, "w") as rec_file:
                json.dump(existing_data, rec_file, indent=2)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant