Skip to content

feat(api): namespace wide disable/enable/stop/delete/update/ready operations on processes#385

Closed
dzmitry-lahoda wants to merge 2 commits into
F1bonacc1:mainfrom
dzmitry-lahoda-forks:dz/6
Closed

feat(api): namespace wide disable/enable/stop/delete/update/ready operations on processes#385
dzmitry-lahoda wants to merge 2 commits into
F1bonacc1:mainfrom
dzmitry-lahoda-forks:dz/6

Conversation

@dzmitry-lahoda

@dzmitry-lahoda dzmitry-lahoda commented Aug 25, 2025

Copy link
Copy Markdown
Contributor

I did not moved mutex from per process to per "namespace". So should I? Currently we accept concurrency to some degree. We could improve concurrency by non erroring idempotent (delete deleted), but seems current code will need a lot of refactor.

Also there is no good split for 5xx vs 4xx errors - because low level code does not do it nor other APIs.

For me both acceptable, but imho there should be task to make errors more structured as go can later for whole API.

Add to API only, it is up to later PR make it CLI/ Go client if deemed useful, so better to be do correct errors first.
Let settle semantics first in API to keep PR smallest possible.

@dzmitry-lahoda dzmitry-lahoda changed the title feat(api): namespace wide disable/enable/stop operations on processes feat(api): namespace wide disable/enable/stop/delete/post operations on processes Aug 25, 2025
@dzmitry-lahoda dzmitry-lahoda changed the title feat(api): namespace wide disable/enable/stop/delete/post operations on processes feat(api): namespace wide disable/enable/stop/delete/update operations on processes Aug 26, 2025
@dzmitry-lahoda dzmitry-lahoda marked this pull request as ready for review August 26, 2025 12:05
@dzmitry-lahoda

Copy link
Copy Markdown
Contributor Author

As for testing, I will do Rust CLI and write Rust tests in our project. If any issues will return with PR.

@dzmitry-lahoda

dzmitry-lahoda commented Aug 27, 2025

Copy link
Copy Markdown
Contributor Author

@F1bonacc1 I do not think it can be merged, so some design could be discussed?

Why something like this to be in Process Compose.

  • to use PC remotely, behind some automation, in tests (or other transient jobs), seems need something to be. so it is not one user use case, but agenda for many. i provided design and impl of one of option.
  • making own things on client side, can end up build own mutex covering whole PC OpenAPI methid, that is harder than should be and harder to maintain. providing what API can as part of Go and CLI clients later will need even more wrapping boiler plate. so making outside is hard.
  • error handling. i feel that error handling to be improved (made more typed overtime). changes can only start in PC code, not string contains by outside code.

@F1bonacc1 F1bonacc1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think this is a good idea.
This make NSs a more important concept in the PC than it is today.
Thanks for the initiative.

I left you a few comments about the implementation.
Most of them are related to the business logic being implemented in the API instead of the project runnner.

Can you please also add some tests? I am not sure that there won't be any race issues. Check with make testrace.

Comment thread src/api/pc_api.go Outdated
Comment thread src/api/pc_api.go
Comment thread src/api/pc_api.go
Comment thread src/api/pc_api.go
@dzmitry-lahoda

Copy link
Copy Markdown
Contributor Author

This make NSs a more important concept in the PC than it is today.

Yeah, people can add more things too later. For example TUI grouping or depends_on namespace or like.

Can you please also add some tests? I am not sure that there won't be any race issues. Check with make testrace.

Ok will do tests.

@dzmitry-lahoda

Copy link
Copy Markdown
Contributor Author

@F1bonacc1 are tests for such changes usually written on api level or on project runner level?

@dzmitry-lahoda dzmitry-lahoda force-pushed the dz/6 branch 3 times, most recently from e707b22 to b7a311a Compare August 28, 2025 11:46
@dzmitry-lahoda

Copy link
Copy Markdown
Contributor Author

As of now "testing" on API(OpenAPI) level via Rust in our test runner codebase. Will send some patches if any after.

@F1bonacc1

Copy link
Copy Markdown
Owner

@F1bonacc1 are tests for such changes usually written on api level or on project runner level?

Project runner level. See system_test.go. We want to check concurrency issues (make testrace).

@sonarqubecloud

Copy link
Copy Markdown

@dzmitry-lahoda

Copy link
Copy Markdown
Contributor Author

need to add namespace ready after #396, so that I can make test(test = namespace) setup wait ready

@dzmitry-lahoda dzmitry-lahoda changed the title feat(api): namespace wide disable/enable/stop/delete/update operations on processes feat(api): namespace wide disable/enable/stop/delete/update/ready operations on processes Sep 3, 2025
@dzmitry-lahoda

Copy link
Copy Markdown
Contributor Author

ready seems hard to define. so just need to return 404 when there is no such namespace at least for now.

impl of not ready or other can be decided by client (handling status code + state + config).

fixes to stop and version

regenerate api

drafts of namespace ops

API only

clean

probably final

no cli

rename add ns to add pcs

moved logic in pc runner

fixed cppst qualit gate

seems ok

docs
@sonarqubecloud

Copy link
Copy Markdown

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