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

cherrypick DYN-6728 feature flags should be controlled by no network mode when c… #14980

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

mjkkirschner
Copy link
Member

cp:
#14975

…reating a dynamo model directly. (DynamoDS#14975)

* check network mode

* align disable and networkmode

* add small tests

* remove todo

* review comments

* another test

// If user skipped analytics from assembly config, do not try to launch the analytics client
// or the feature flags client for web traffic reason.
if (!IsServiceMode && !areAnalyticsDisabledFromConfig && !Analytics.DisableAnalytics)
if (!IsServiceMode && !areAnalyticsDisabledFromConfig && !Analytics.DisableAnalytics && !NoNetworkMode)
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought Analytics.DisableAnalytics = NoNetworkMode || Analytics.DisableAnalytics; above is already covering it because !Analytics.DisableAnalytics is used here?

Copy link
Member Author

Choose a reason for hiding this comment

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

it does cover it, but I think moving forward we really should be using checking NoNetworkMode separately everywhere - and I will file a task to look into that - as this bug shows, not everyone uses the same code paths when starting Dynamo if they write their own entry points.

@mjkkirschner mjkkirschner merged commit 4c94153 into DynamoDS:RC3.0.4_master Feb 27, 2024
20 checks passed
@mjkkirschner mjkkirschner deleted the 304nn branch February 27, 2024 16:55
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