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

Fixes #26335: Port RestApiAccount api endpoint to zio-json #6173

Draft
wants to merge 1 commit into
base: branches/rudder/8.3
Choose a base branch
from

Conversation

fanf
Copy link
Member

@fanf fanf commented Feb 11, 2025

https://issues.rudder.io/issues/26335

WIP status at 2025-02-14

  • The backend is working accordingly to what we want, API tests work
  • I didn't clean-up things for now: in particular RestApiAccounts must be deleted (butI did clean-up things related to the old format)
  • elm does not work, at least because elm side isn't able to decode rfc3339 datetime and is expected an other format for the date pickler.
    • there will be some other inconsistencies, since the API input for update / regenerate token / create token have changed a bit
    • I don't know if there's other things not working since for now, it's blocked here.

On the backend side, the most notable change are that we have different data structures for different action and return values.
In particular:

  • create and update are different, since when creating we need to be able to optionnaly choose ID and generate or not a token,
  • there is a difference between action that can expose a token (create, regenerate) and the one that just query information to avoid displaying a token. There is also a difference between the ClearTextSecret and the ApiToken (hashed).

@fanf fanf force-pushed the arch_26335/port_restapiaccount_api_endpoint_to_zio_json branch from 45f1299 to 8260a11 Compare February 14, 2025 17:48
@@ -72,10 +72,10 @@ class RestApiAccounts(

// used in ApiAccounts snippet to get the context path
// of that service
val relativePath: List[String] = "secure" :: "apiaccounts" :: Nil
Copy link
Member Author

Choose a reason for hiding this comment

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

this file need to be deleted once we have checked the different use cases

@@ -1766,68 +1765,6 @@ object RudderConfigInit {
)
}

lazy val directiveApiService14 = {
Copy link
Member Author

Choose a reason for hiding this comment

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

we need to move them inline to avoid dreaded Platform restriction: a parameter list's length cannot exceed 254.

@@ -58,7 +60,7 @@ object DateFormaterService {
ZonedDateTime.ofInstant(java.time.Instant.ofEpochMilli(d.getMillis), ZoneId.of(d.getZone.getID, ZoneId.SHORT_IDS))
}

object json {
trait DateTimeCodecs {
Copy link
Member Author

Choose a reason for hiding this comment

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

make it a trait to ease composition with other codes

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.

1 participant