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

E4K V1A3 check updates #14

Merged
merged 12 commits into from
Aug 14, 2023
Merged

E4K V1A3 check updates #14

merged 12 commits into from
Aug 14, 2023

Conversation

c-ryan-k
Copy link
Member

@c-ryan-k c-ryan-k commented Jul 28, 2023

Adds support for E4K v1a3 API:

  • sets Active API to v1a3
  • adds 'diagnostic' property check to broker details
  • changes broker_diagnostic checks to diagnostic_service checks and updates logic / evals

This project has adopted the Microsoft Open Source Code of Conduct. For more information see the Code of Conduct FAQ or contact opencode@microsoft.com with any additional questions or comments.

Thank you for contributing to Azure Edge tooling!

This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

Intent for Production

  • It is expected that pull requests made to default or core branches such as dev or main are of production grade. Corollary to this, any merged contributions to these branches may be deployed in a public release at any given time. By checking this box, you agree and commit to the expected production quality of code.

Basic expectations

  • If introducing new functionality or modified behavior, are they backed by unit and/or integration tests?
  • In the same context as above are command names and their parameter definitions accurate? Do help docs have sufficient content?
  • Have all the relevant unit and integration tests pass? i.e. pytest <project root> -vv. Please provide evidence in the form of a screenshot showing a succesful run of tests locally OR a link to a test pipeline that has been run against the change-set.
  • Have linter checks passed using the .pylintrc and .flake8 rules? Look at the CI scripts for example usage.
  • Have extraneous print or debug statements, commented out code-blocks or code-statements (if any) been removed from the surface area of changes?
  • Have you made an entry in HISTORY.rst which concisely explains your user-facing feature or change?

Azure Edge CLI maintainers reserve the right to enforce any of the outlined expectations.

A PR is considered ready for review when all basic expectations have been met (or do not apply).

c-ryan-k added 8 commits July 28, 2023 09:05
renamed chainCount to partitions

Added backend workers display
Broker Diagnostics resource changed to Diagnostic Services resource

Moved broker diagnostics details into individual broker detail checks

Removed diagnostic ref check

Changed missing broker diagnostics from skip to warning
Added target conditions to diagnostics service eval

Set error status if count of resources is not 1

Added evals for each diagnostic service resource

adjusted indendation of broker diagnostics logic
@c-ryan-k c-ryan-k marked this pull request as ready for review August 7, 2023 23:20
@c-ryan-k c-ryan-k requested a review from digimaun as a code owner August 7, 2023 23:20
@c-ryan-k c-ryan-k requested review from Elsie4ever and vilit1 August 7, 2023 23:21
diag_service_spec_log_level = diag_service_resource_spec.get("logLevel")
diag_service_spec_max_data_storage_size = diag_service_resource_spec.get("maxDataStorageSize")
diag_service_spec_metrics_port = diag_service_resource_spec.get("metricsPort")
diag_service_spec_stale_data_timeout = diag_service_resource_spec.get("staleDataTimeoutSeconds")
Copy link
Contributor

Choose a reason for hiding this comment

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

should we have none checks (since the get will return None if the key doesn't exist and the message is still printed

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's a bad idea to have defaults, though in this case I'd want to steer away from quantitative defaults like "0", but I think since these are just simple outputs (and not tracked as a target / required value) that something like None is ok in place of N/A or another filler value
image

Copy link
Contributor

Choose a reason for hiding this comment

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

as long as
"Data Export Frequency: [bright_blue]None[/bright_blue] seconds"

is ok, then everything is good

diagnostic_service_count = len(diagnostics_service_resources)

service_count_status = CheckTaskStatus.success.value
service_status_color = "green"
Copy link
Contributor

Choose a reason for hiding this comment

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

future task should put these colors into an enum to help explain what they mean:
success = green
warning = yellow
info/idk = blue

target_name=target_brokers,
display=Padding(
backend_workers_desc.format(backend_workers_colored),
(0, 0, 0, 16),
Copy link
Contributor

Choose a reason for hiding this comment

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

future task of putting these tuples into enums (assuming everything follows (0,0,0,x)

@c-ryan-k c-ryan-k requested a review from vilit1 August 14, 2023 18:26
@vilit1
Copy link
Contributor

vilit1 commented Aug 14, 2023

lgtm - I will let you squash and merge

@c-ryan-k c-ryan-k merged commit eefa565 into Azure:dev Aug 14, 2023
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