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

Refactor Docker container launching code #43

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

lpsinger
Copy link
Member

Put subprocess and parent process code in the same file.

@lpsinger lpsinger requested a review from Courey June 25, 2024 16:19
runDocker.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@Courey Courey left a comment

Choose a reason for hiding this comment

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

please change the launchDocker function to use the updated types

@Courey
Copy link
Contributor

Courey commented Jun 25, 2024

you also need to update the import from the index

Did you test this change?

@lpsinger lpsinger force-pushed the refactor-launch-docker branch from b66ab7f to 3e5c9fd Compare June 25, 2024 20:00
runDocker.ts Outdated Show resolved Hide resolved
lpsinger added a commit to lpsinger/architect-plugin-search that referenced this pull request Jun 25, 2024
This should help catch issues like
nasa-gcn#43 (comment).
@lpsinger lpsinger force-pushed the refactor-launch-docker branch from 3e5c9fd to 490d25d Compare June 25, 2024 20:27
lpsinger added a commit to lpsinger/architect-plugin-search that referenced this pull request Jun 25, 2024
This should help catch issues like
nasa-gcn#43 (comment).
lpsinger added a commit that referenced this pull request Jun 25, 2024
This should help catch issues like
#43 (comment).
@lpsinger
Copy link
Member Author

Did you test this change?

I tried adding a unit test (#45) but it doesn't run the sandbox teardown code even for Elasticsearch. Any ideas?

@Courey
Copy link
Contributor

Courey commented Jun 27, 2024

Did you test this change?

I tried adding a unit test (#45) but it doesn't run the sandbox teardown code even for Elasticsearch. Any ideas?

I meant by running both docker and binary locally at least, but unit tests are good too! Very interesting that it doesn't run the teardown code! I don't have any ideas about why off the top of my head. I can look into why, but it may take a couple of days because I'm in the middle of two other tickets.

Put subprocess and parent process code in the same file.
@lpsinger lpsinger force-pushed the refactor-launch-docker branch from 490d25d to 85064ee Compare July 2, 2024 18:19
@lpsinger lpsinger requested a review from Courey July 2, 2024 18:21
@lpsinger
Copy link
Member Author

lpsinger commented Jul 2, 2024

@Courey, please review when you get a chance.

@lpsinger lpsinger merged commit 388b4fc into nasa-gcn:main Jul 3, 2024
10 checks passed
@lpsinger lpsinger deleted the refactor-launch-docker branch July 3, 2024 15:41
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