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

[Databricks E2E] Refactor code and log messages to stderr #962

Merged

Conversation

elenaterenzi
Copy link
Contributor

@elenaterenzi elenaterenzi commented Dec 17, 2024

Type of PR

  • Code changes

Purpose

  • Introduce a log function that makes sure that messages are always logged to stderr. From now on we should avoid using echo to send (human-readable) messages to console.
    • The function also takes care of changing the style of the displayed text using colors (using existing function print_style)
  • code refactoring: changed some function names to keep the same naming convention
  • commented a number of set -o xtrace that are printing tokens to console
  • modified .Dockerfile to include a bicep upgrade to remove warnings

Does this introduce a breaking change? If yes, details on what can break

No breaking change, but moving forward we should use this new function to log messages to console

Author pre-publish checklist

  • Added test to prove my fix is effective or new feature works
  • No PII in logs
  • Made corresponding changes to the documentation

Validation steps

  • ...

Issues Closed or Referenced

@elenaterenzi elenaterenzi changed the base branch from main to e2e/databricks/parking-sensors-V1 December 17, 2024 18:21
@elenaterenzi elenaterenzi changed the title Refactor code and log messages to stderr [Databricks E2E] Refactor code and log messages to stderr Dec 18, 2024
@elenaterenzi elenaterenzi self-assigned this Dec 18, 2024
Copy link
Contributor

@LiliamLeme LiliamLeme left a comment

Choose a reason for hiding this comment

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

Cool log function. I left just one comment while loading the env variables in the bash file., approving so it will not block you

e2e_samples/parking_sensors/scripts/init_environment.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@ydaponte ydaponte left a comment

Choose a reason for hiding this comment

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

I'm just think we should delete the trace instead of commenting not to have inactive code checked in - but maybe there is a reason why you want to keep it commented? Other than that - GREAT job! I'm thrilled that we have this function now!

@elenaterenzi elenaterenzi merged commit aba8fa9 into e2e/databricks/parking-sensors-V1 Dec 18, 2024
1 check passed
@elenaterenzi elenaterenzi deleted the elena/log-function-bash branch December 18, 2024 15:48
ydaponte pushed a commit that referenced this pull request Dec 19, 2024
* initial changes

* disable debug

* typo

* reference common.sh in init_environment and remove when not needed

* remove commented code

* remove commented code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

code refactor: log function
3 participants