Override env vars #241
Merged
Override env vars #241
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mlohvynenko
reviewed
Jan 23, 2025
include/aos/sm/launcher/instance.hpp
Outdated
| const EnvVarsArray& GetOverrideEnvVars() const { return mOverrideEnvVars; }; | ||
|
|
||
| /** | ||
| * Sets |
There was a problem hiding this comment.
Suggested change
| * Sets | |
| * Sets instance override env vars. |
mlohvynenko
reviewed
Jan 23, 2025
include/aos/sm/launcher/instance.hpp
Outdated
| /** | ||
| * Returns instance override env vars. | ||
| * | ||
| * @return EnvVarsArray. |
There was a problem hiding this comment.
Suggested change
| * @return EnvVarsArray. | |
| * @return const EnvVarsArray&. |
mlohvynenko
reviewed
Jan 23, 2025
src/sm/launcher/launcher.cpp
Outdated
| mCurrentInstances.RemoveIf( | ||
| [&info](const Instance& instance) { return instance.InstanceID() == info.mInstanceID; }); | ||
| // Skip already stopped instances | ||
| if (GetInstance(info.mInstanceID) == mCurrentInstances.end()) { |
There was a problem hiding this comment.
Looks like a typo:
I've expected the next condition:
Suggested change
| if (GetInstance(info.mInstanceID) == mCurrentInstances.end()) { | |
| if (GetInstance(info.mInstanceID) != mCurrentInstances.end()) { |
al1img
force-pushed
the
override_env_vars
branch
from
ddd3606 to
8e6f4c2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #241 +/- ##
===========================================
- Coverage 86.56% 83.58% -2.98%
===========================================
Files 85 157 +72
Lines 7448 13712 +6264
Branches 0 1709 +1709
===========================================
+ Hits 6447 11461 +5014
- Misses 1001 2251 +1250 ☔ View full report in Codecov by Sentry. |
mykola-kobets-epam
approved these changes
Jan 30, 2025
mlohvynenko
reviewed
Jan 30, 2025
src/sm/launcher/launcher.cpp
Outdated
|
|
||
| Error Launcher::UpdateInstancesEnvVars() | ||
| { | ||
| auto restartInstances = SharedPtr<Array<InstanceData>>(&mAllocator, new (&mAllocator) InstanceDataStaticArray()); |
There was a problem hiding this comment.
Can we use make shared?
Suggested change
| auto restartInstances = SharedPtr<Array<InstanceData>>(&mAllocator, new (&mAllocator) InstanceDataStaticArray()); | |
| auto restartInstances = MakeShared<InstanceDataStaticArray>(mAllocator); |
mlohvynenko
approved these changes
Jan 30, 2025
al1img
force-pushed
the
override_env_vars
branch
from
12f45f9 to
3ffd482
Compare
Instance should be remove in current instances map, as this method will be used for restart instances as well. Signed-off-by: Oleksandr Grytsov <[email protected]>
Signed-off-by: Oleksandr Grytsov <[email protected]>
Signed-off-by: Oleksandr Grytsov <[email protected]>
Signed-off-by: Oleksandr Grytsov <[email protected]>
Signed-off-by: Oleksandr Grytsov <[email protected]>
Using value in list node requires list element to always have default constructor. It is not possible in some cases. Use buffer and cast it to value to properly handle items without default constructor. Signed-off-by: Oleksandr Grytsov <[email protected]>
During process instances (namely stop instances) concurrently, we remove instances from array. It leads to array rearrangement, as result currently processed instances may point to wrong place. Use list instead of array to avoid this issue. Signed-off-by: Oleksandr Grytsov <[email protected]>
al1img
force-pushed
the
override_env_vars
branch
from
3ffd482 to
36b8ae5
Compare
|
mlohvynenko
approved these changes
Jan 31, 2025
MykolaSolyanko
approved these changes
Jan 31, 2025
mlohvynenko
approved these changes
Jan 31, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.