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

naming of operation parameters mismatch #3853

Open
jsnoble opened this issue Dec 5, 2024 · 2 comments
Open

naming of operation parameters mismatch #3853

jsnoble opened this issue Dec 5, 2024 · 2 comments

Comments

@jsnoble
Copy link
Member

jsnoble commented Dec 5, 2024

We have a naming style used in operation that is globally recognized by the teraslice system that usually starts with an _, for example we have _op, _encoding, and _dead_letter_action. However we have two other configurations that don't follow this naming style, its connection and api_name. These are important settings that have meaning in our configurations if set and it can be confusing if some require an _ while others don't.

I would recommend making them all consistent, though this would require a lot of changes so I wanted to write this ticket for your thoughts

@godber
Copy link
Member

godber commented Dec 5, 2024

It's suggestions like this that make me think we can take some time and think through how we change _op and be open to more drastic changes that would not be backwards compatible if they make sense to do. That's why I keep saying things like "we can make this a 3.0 thing".

Trying to understand what you're saying, the OpConfig type:

export interface OpConfig {
/** The name of the operation */
_op: string;
/**
* Used for specifying the data encoding type when using `DataEntity.fromBuffer`.
*
* @default `json`.
*/
_encoding?: DataEncoding;
/**
* This action will specify what to do when failing to parse or transform a record.
* The following builtin actions are supported:
* - "throw": throw the original error
* - "log": log the error and the data
* - "none": (default) skip the error entirely
*
* If none of the actions are specified it will try and
* use a registered Dead Letter Queue API under that name.
* The API must be already be created by a operation before it can used.
*/
_dead_letter_action?: DeadLetterAction;
[prop: string]: any;
}

is:

export interface OpConfig {
    _op: string;
    _encoding?: DataEncoding;
    _dead_letter_action?: DeadLetterAction;
    [prop: string]: any;
}

I hadn't really ever realized what the _ meant but I guess it means "this property is present in ALL Operations"? I'll go dig up connection and api_name ... I guess ... what does it mean that those two aren't in THAT type?

@jsnoble
Copy link
Member Author

jsnoble commented Dec 6, 2024

Thats kinda my point that connection and api_name are not in there, it was missed, as for your comment about but I guess it means "this property is present in ALL Operations"? , it not that its necessarily in all operations (though for the most part it is), but that its a configuration meant for teraslice itself and is a recognized setting that can be on every op or api instead of it being a unique parameter for only a specific op or api.

For example, connection is a parameter used by teraslice apis to determine how to make a client from the connectors and api_name is another parameter that is used by teraslice apis to determine and tag which api to use for a given operation. Its all teraslice specific behaviour and don't really have anything to do with the unique parameters of an operation or api

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants