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

Telemetry off env parameter #286

Closed
wants to merge 2 commits into from
Closed

Telemetry off env parameter #286

wants to merge 2 commits into from

Conversation

thomas-gerber
Copy link
Contributor

Description

Adds a parameter that disable telemetry completely before starting Faros CE.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

(Delete what does not apply)

  • Have you checked to there aren't other open Pull Requests for the same update/change?
  • Have you lint your code locally before submission?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully run tests with your changes locally?

@thomas-gerber
Copy link
Contributor Author

Before merging, I would like to understand why init failed for our user - it shouldn't have AFAICT: https://faroscommunity.slack.com/archives/C0335HMN2NQ/p1677263489918959

@@ -307,6 +307,7 @@ async function main(): Promise<void> {
.requiredOption('--airbyte-destination-hasura-url <string>')
.requiredOption('--hasura-admin-secret <string>')
.option('--force-setup')
.option('--telemetry-off')
Copy link
Contributor

Choose a reason for hiding this comment

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

Commander recommends --no-telemetry instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noted

Copy link
Contributor

Choose a reason for hiding this comment

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

and then you can call you member in options class simply telemetry

@thomas-gerber
Copy link
Contributor Author

Closing for now without merging because:

  1. we haven't root-caused why the error isn't caught (it is probably not super mysterious for someone familiar with async calls)
  2. we are re-doing the whole init anyway for Airbyte v40

I'll keep the branch around until that work is done just in case someone else faces this issue.

@github-actions github-actions bot locked and limited conversation to collaborators May 8, 2023
@thomas-gerber thomas-gerber deleted the telemetry_off branch May 20, 2023 00:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants