-
Notifications
You must be signed in to change notification settings - Fork 27
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
[Console] Metadata Migration #756
[Console] Metadata Migration #756
Conversation
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #756 +/- ##
============================================
+ Coverage 68.50% 68.54% +0.04%
+ Complexity 1583 1577 -6
============================================
Files 270 272 +2
Lines 11175 11335 +160
Branches 736 736
============================================
+ Hits 7655 7770 +115
- Misses 3118 3162 +44
- Partials 402 403 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
…etdata-migration Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
@@ -5,7 +5,7 @@ COPY python/Pipfile python/Pipfile.lock ./ | |||
RUN apt -y update | |||
RUN apt -y install python3 python3-pip | |||
RUN pip3 install pipenv | |||
RUN pipenv install --system --deploy --ignore-pipfile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is included because I was getting this error while deploying to aws. It seems odd that I would run into this and no one else would, so it's possible there's another explanation.
cc: @peternied
#10 [ 6/10] RUN pipenv install --system --deploy --ignore-pipfile
#10 0.471 Usage: pipenv install [OPTIONS] [PACKAGES]...
#10 0.471
#10 0.471 ERROR:: --system is intended to be used for Pipfile installation, not installation of specific packages. Aborting.
#10 0.471 See also: --deploy flag.
#10 ERROR: process "/bin/sh -c pipenv install --system --deploy --ignore-pipfile" did not complete successfully: exit code: 2
------
> [ 6/10] RUN pipenv install --system --deploy --ignore-pipfile:
0.471 Usage: pipenv install [OPTIONS] [PACKAGES]...
0.471
0.471 ERROR:: --system is intended to be used for Pipfile installation, not installation of specific packages. Aborting.
0.471 See also: --deploy flag.
------
Dockerfile:8
--------------------
6 | RUN apt -y install python3 python3-pip
7 | RUN pip3 install pipenv
8 | >>> RUN pipenv install --system --deploy --ignore-pipfile
9 |
10 |
--------------------
ERROR: failed to solve: process "/bin/sh -c pipenv install --system --deploy --ignore-pipfile" did not complete successfully: exit code: 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you have this commit when you got that error? #753
TrafficCapture/dockerSolution/src/main/docker/docker-compose.yml
Outdated
Show resolved
Hide resolved
MigrationType = Enum('MigrationType', ['OSI_HISTORICAL_MIGRATION']) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lewijacn -- neither the file it's being imported from nor the enum referenced exist anymore. I added it locally for the time being, because I don't think this is potentially changeable data at this point. Post-demo, we should 1/ add tests to the api so we don't accidentally break it again, 2/ figure out what this should be calling.
Signed-off-by: Mikayla Thompson <thomika@amazon.com>
fff7c41
to
53095aa
Compare
@@ -8,7 +8,7 @@ if [ -z "$MIGRATION_SERVICES_YAML_PARAMETER" ]; then | |||
fi | |||
|
|||
# Retrieve the parameter value from AWS Systems Manager Parameter Store | |||
PARAMETER_VALUE=$(aws ssm get-parameters --names "$MIGRATION_SERVICES_YAML_PARAMETER" --query "Parameters[0].Value" --output text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this still after installing awscli?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I seem to, but I don't know if that's entirely true if it works for other people without it.
Based on the docs, it kind of seems like we need to either do this, or to do pipenv shell
before we run any commands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting... i wasn't encountering issues but i'm not opposed to this change either
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, please take a look at the comment i left before merging
Description
Add metadata migration, for both local filesystem snapshots in docker & s3 snapshots in ECS (it should also work in s3 in docker and filesystem in ECS, as long as all permissions, etc. are set up correctly, but I haven't tested those permutations).
Mini-walkthroughs from testing of each option at the bottom.
Issues Resolved
MIGRATIONS-1791
Testing
Pretty good unit test coverage, plus lots of manual tests.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.
In docker:
On ECS