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

arkouda_server refactor with external integration #1602

Conversation

hokiegeek2
Copy link
Contributor

@hokiegeek2 hokiegeek2 commented Jul 19, 2022

This PR basically extends the arkouda_server_refactor PR that was merged last week, encapsulating logic to register Arkouda with external systems--in this PR, specifically as a Kubernetes service) upon Arkouda startup and deregister Arkouda from Kubernetes upon receipt of the ak.shutdown() client request.

This PR consists of the following:

  1. ExternalIntegrationServerDaemon class within the ServerDeamon module
  2. ExternalIntegration module that encapsulates generalized registerWithExternalSystem and deregisterFromExternalSystem functions that are invoked from the ExternalIntegrationServerDaemon.run() function along with functions that specifically register/deregister Arkouda with/from Kubernetes
  3. adds ExternalIntegrationServerDaemon to the getServerDaemon factory function that returns an ArkoudaServerDaemon implementation per a CLI arg
  4. EXTERNAL_INTEGRATION.md file that details Arkouda external integration

@hokiegeek2 hokiegeek2 changed the title Arkouda server refactor with external integration arkouda_server refactor with external integration Jul 19, 2022
Copy link
Contributor

@Ethan-DeBandi99 Ethan-DeBandi99 left a comment

Choose a reason for hiding this comment

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

This looks good. For clarity, it would probably be worthwhile documenting why this PR is separate from the standard refactor. The reasoning was clear after we spoke, but it would be good to document.

Posting as comment since this is still a draft

@hokiegeek2 hokiegeek2 marked this pull request as ready for review August 10, 2022 17:12
@hokiegeek2
Copy link
Contributor Author

@mhmerrill @reuster986 @pierce314159 @Ethan-DeBandi99 @joshmarshall1 this is now ready for review. I merged in the final arkouda_server-ServerDaemon refactor, consolidated all register/deregister logic in the ExternalIntegration module and added a EXTERNAL_INTEGRATION.md file linked to the main README.md detailing how things work.

Copy link
Contributor

@Ethan-DeBandi99 Ethan-DeBandi99 left a comment

Choose a reason for hiding this comment

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

I think this looks good to go.

@Ethan-DeBandi99 Ethan-DeBandi99 merged commit 02ccf1a into Bears-R-Us:master Aug 10, 2022
@hokiegeek2 hokiegeek2 deleted the arkouda_server_refactor_external_integration branch September 7, 2022 18:27
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.

4 participants