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

cleanup unused functions from management service #24

Closed
iakkus opened this issue May 29, 2020 · 6 comments
Closed

cleanup unused functions from management service #24

iakkus opened this issue May 29, 2020 · 6 comments
Assignees
Labels
finished When a bug is fixed or a feature has been implemented improvement Improvements to an existing component

Comments

@iakkus
Copy link
Member

iakkus commented May 29, 2020

[Environment]: bare metal, Kubernetes (maybe)
[Known affected releases]: master (includes all releases)

The management service has the following functions that are either never called or don't do anything meaningful:

  1. clearWorkflowLog (does nothing)
  2. clearAllWorkflowLogs (does nothing)
  3. prepareWorkflowLog (does nothing)
  4. prepareAllWorkflowLogs (does nothing)
  5. retrieveWorkflowLog (never called, provides functionality of subset of retrieveAllWorkflowLogs)

retrieveAllWorkflowLogs returns all workflow related logs and it is being used throughout GUI as well as SDK. One can modify it to use an optional parameter to return only a particular log type if needed, making retrieveWorkflowLog fully redundant.

Besides the management service, the GUI needs to be updated to skip the redundant calls to prepareAllWorkflowLogs and directly call retrieveAllWorkflowLogs. SDK already uses retrieveAllWorkflowLogs directly.

This would reduce the resource usage of the management service sandbox.

@iakkus iakkus added the improvement Improvements to an existing component label May 29, 2020
@manuelstein
Copy link
Collaborator

Backwards compatibility
The management service constitutes our API, so removing functions (without a deprecation period) means we're breaking backward compatibility. That's okayish as we haven't reached out first major release, just pointing it out.

clearWorkflowLog IIUC is a placeholder that hasn't been implemented yet with Elasticsearch. Otherwise, I think we need a solution to manage log data in Elasticsearch. AS a side note, we should consider removing log data when removing a function.

@iakkus
Copy link
Member Author

iakkus commented May 29, 2020

You mean, if somebody has written a piece of code that utilizes the Management service APIs (besides our code, like the SDK)? Sure, we could add the appropriate responses.

clearWorkflowLog and clearAllWorkflowLogs: we can modify them to indeed the delete the logs from Elasticsearch, but I think it might be better if we could automatically delete all logs after a certain amount of time (e.g., 30 days). For example,

@paarijaat, what do you think?

@iakkus iakkus self-assigned this May 29, 2020
@iakkus iakkus added the in progress This issue is already being fixed label May 29, 2020
@iakkus
Copy link
Member Author

iakkus commented May 29, 2020

Implementation started on branch feature/cleanup_management.

@iakkus
Copy link
Member Author

iakkus commented Jun 1, 2020

AS a side note, we should consider removing log data when removing a function.

I think perhaps removing the logs of a deleted workflow would make more sense.

@manuelstein
Copy link
Collaborator

Yes, of course, as we're not executing functions but workflows.

@iakkus
Copy link
Member Author

iakkus commented Jun 2, 2020

New issue to track deletion of workflow logs: #25

@iakkus iakkus closed this as completed Jun 3, 2020
@iakkus iakkus added finished When a bug is fixed or a feature has been implemented and removed in progress This issue is already being fixed labels Jun 9, 2020
@iakkus iakkus mentioned this issue Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
finished When a bug is fixed or a feature has been implemented improvement Improvements to an existing component
Projects
None yet
Development

No branches or pull requests

2 participants