RelationalAI / rai-sdk-python

The RelationalAI Software Development Kit (SDK) for Python.
Apache License 2.0
17 stars 4 forks source link

Refactored the tests to use the github action #111

Closed miazamrai closed 2 years ago

miazamrai commented 2 years ago

Moved the test code to be called as github action so that the source for testing via the python sdk resides on a single place. It will allow the consumers to absorb any changes made to the test logic.

miazamrai commented 2 years ago

@larf311 @NRHelmi We need to do same kind of stuff for all the other SDKs, that is, moving the test code to a github action. It would be nice that we use the same folder github folder structure for all the SDKs.

NRHelmi commented 2 years ago

@miazamrai so we don't need to redefine action.yml in raicode right ?

miazamrai commented 2 years ago

@miazamrai so we don't need to redefine action.yml in raicode right ?

Yes, the other repository workflow will checkout the sdk and directly run the action from the sdk’s action folder.

larf311 commented 2 years ago

Don't you need to (optionally) pass in the engine version?

miazamrai commented 2 years ago

Don't you need to (optionally) pass in the engine version?

it will be like this. https://github.com/RelationalAI/raicode/blob/276b5325acb9daca2d453903f3712c838a228fd9/.github/workflows/raicloud-bench-sdktest-all.yml#L27

larf311 commented 2 years ago

Is there a reason not to make the engine version (or perhaps custom headers) an explicit input? It would make the action self documenting. As it is now, callers of this action need to know the special environment variable to set.

miazamrai commented 2 years ago

Is there a reason not to make the engine version (or perhaps custom headers) an explicit input? It would make the action self documenting. As it is now, callers of this action need to know the special environment variable to set.

I opted the env variable approach for the following reasons,

  1. The custom headers are treated by the SDKs as env var.
  2. Same github action is being used by both the SDK's CI build and the other repositories and it is not the requirement for the SDK's CI.
  3. Even if the engine version is sent as an optional input param, the action needs to export it as an env var with the right key/value pair.
  4. Wanted to keep the engine version details (header details) internal to the rai private repository because we don't want to expose the internal information to public since SDK is a public repo.
miazamrai commented 2 years ago

@larf311 If you are agree with the current approach then I will merge this to main.

larf311 commented 2 years ago
  1. Why does a caller of the action need to know if internally it uses env variables or command line params or a config file?
  2. That's why it would be an optional input
  3. I don't really understand this argument.
  4. You could make the input be custom headers instead of engine version to avoid this, which is probably more robust since it will allow additional functionality such as log levels, etc.
miazamrai commented 2 years ago
  • I don't really understand this argument.

I like #4

miazamrai commented 2 years ago

@larf311 Added the custom headers input param.