Skip to content

Adding some admin functionality#87

Merged
sondove merged 18 commits intonovem-code:mainfrom
sondove:sen/groups_and_jobs
May 12, 2025
Merged

Adding some admin functionality#87
sondove merged 18 commits intonovem-code:mainfrom
sondove:sen/groups_and_jobs

Conversation

@sondove
Copy link
Contributor

@sondove sondove commented May 11, 2025

Here is a first draft for adding jobs, repos, orgs and claims to the python library.

There is definitely room for a refactor or two, but I wanted to get this merged so we can progress on some of the setup work.

I've refactored the share logic, so I suspect we should do some extensive testing here

sondove added 16 commits May 11, 2025 17:35
As an api-first platform it's nice to have some utility code for
managing organisations and groups. This commit adds some basic group
controls
We're trying to harmonize on a `get_share_string()` for shareable
objects, so its nice to have a claim wrapper just to be explicit
This is probably a larger commit than it needs to be, but here we are
trying to create a generalised share that works for both repos and vis.
Copy link
Contributor

@bjornars bjornars left a comment

Choose a reason for hiding this comment

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

Minimal interference with "core" functionality, but it feels a bit vibey and there are definite possibilites for refactoring this and making it way more consise.

Comment on lines +1 to +5
class Claim:
id: str

def __init__(self, id: str) -> None:
self.id = id
Copy link
Contributor

Choose a reason for hiding this comment

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

a bit nicer as a dataclass

return self.api.api_write("/profile/options/mail_verify_spf", val)


class NovemGroupProfile(object):
Copy link
Contributor

Choose a reason for hiding this comment

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

stop with the object inheritance, thats a python2 thing.

@@ -0,0 +1,507 @@
import configparser
Copy link
Contributor

Choose a reason for hiding this comment

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

I would love it if all these basic functionality tests didnt need to mock out the filesystem - but I guess that's a part of the major rewrite that I keep getting stuck on.

from .roles import NovemRoles


class NovemGroupAPI(NovemAPI):
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a lot of copy paste between all these NovemAPI subclasses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, 100%, there is a big refactor waiting to happen here

Comment on lines +487 to +581
requests_mock.register_uri(
"put",
f"{api_root}jobs/{job_id}",
text="",
)

# Config endpoints - POST
requests_mock.register_uri(
"post",
f"{api_root}jobs/{job_id}/config/type",
text="",
)

requests_mock.register_uri(
"post",
f"{api_root}jobs/{job_id}/config/extract",
text="",
)

requests_mock.register_uri(
"post",
f"{api_root}jobs/{job_id}/config/render",
text="",
)

# Create job instance with config dictionary
j = Job(job_id, config_path=config_file, config=config_dict)

# Register GET endpoints to check the config was set via the config dictionary
requests_mock.register_uri(
"get",
f"{api_root}jobs/{job_id}/config/type",
text=job_type,
)

requests_mock.register_uri(
"get",
f"{api_root}jobs/{job_id}/config/extract",
text=job_extract,
)

requests_mock.register_uri(
"get",
f"{api_root}jobs/{job_id}/config/render",
text=job_render,
)

# Verify the config was set through the config dictionary
assert j.config.type == job_type
assert j.config.extract == job_extract
assert j.config.render == job_render

# Test directly using the set method
new_type = "scheduled"
new_extract = "api"
new_render = "table"

requests_mock.register_uri(
"post",
f"{api_root}jobs/{job_id}/config/type",
text="",
)

requests_mock.register_uri(
"post",
f"{api_root}jobs/{job_id}/config/extract",
text="",
)

requests_mock.register_uri(
"post",
f"{api_root}jobs/{job_id}/config/render",
text="",
)

j.config.set({"type": new_type, "extract": new_extract, "render": new_render})

# Update mocks for the new config values
requests_mock.register_uri(
"get",
f"{api_root}jobs/{job_id}/config/type",
text=new_type,
)

requests_mock.register_uri(
"get",
f"{api_root}jobs/{job_id}/config/extract",
text=new_extract,
)

requests_mock.register_uri(
"get",
f"{api_root}jobs/{job_id}/config/render",
text=new_render,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sure there's room for some refactoring here with all these mocks - some fixtures or just some helper functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sure it's obvious, but I just vibed out most of these tests. My goal was to exercise some of the getter/setter patterns to support the more concise grammar I was looking for

sondove added 2 commits May 12, 2025 12:52
Guess this bug shows the value of the AI generated test cases to be
close to zero...
@sondove sondove merged commit 987544d into novem-code:main May 12, 2025
7 checks passed
@sondove sondove deleted the sen/groups_and_jobs branch May 12, 2025 14:01
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