Skip to content

Refactor groups classes (CADC-14501)#201

Merged
pdowler merged 11 commits intoopencadc:mainfrom
andamian:CADC-14501
Jan 21, 2026
Merged

Refactor groups classes (CADC-14501)#201
pdowler merged 11 commits intoopencadc:mainfrom
andamian:CADC-14501

Conversation

@andamian
Copy link
Contributor

@andamian andamian commented Jan 6, 2026

No description provided.

@andamian andamian requested a review from pdowler January 6, 2026 19:11
Copy link
Member

@pdowler pdowler left a comment

Choose a reason for hiding this comment

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

I would put the cadc-rest based groups code into ac directly and not in the library, in a package named org.opencadc.ac (for now). It's essentially a fork on that code, moved to new package.... you could just leave the old library code alone for now. I don't think the diffs will be useful enough.

It is harder than necessary to put action code into a lib and make them configurable in a service and there is no need here.

Copy link
Member

@pdowler pdowler left a comment

Choose a reason for hiding this comment

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

Couple more general comments after trying to deploy and run integration tests:

There is no README documenting the required config, specifically the new file ac.properties. Please add and follow the standard form from other locations. You will need to remove the part about IdentityManager config for now since ac has a hard-coded internal one.

@pdowler pdowler merged commit bada971 into opencadc:main Jan 21, 2026
1 check passed
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