Skip to content

Conversation

@abhaasgoyal
Copy link
Contributor

@abhaasgoyal abhaasgoyal commented Apr 4, 2025

Related to #63

Implement endpoint for creating a new model output (POST method) for a given Experiment ID + Model Profile ID

Add Github secrets for the following (following from the existing model output id)

  1. MEORG_MODEL_OUTPUT_NAME - If no environment variable is provided for pytest, "meorg-client-model-output" is used
  2. MEORG_MODEL_PROFILE_ID
  3. MEORG_EXPERIMENT_ID

@abhaasgoyal abhaasgoyal force-pushed the 3-model-output-endpoint branch from 31a3622 to 0c944eb Compare April 15, 2025 05:17
@abhaasgoyal abhaasgoyal requested a review from bschroeter April 15, 2025 05:17
@abhaasgoyal abhaasgoyal marked this pull request as ready for review April 15, 2025 05:17
@abhaasgoyal abhaasgoyal changed the title 3 model output endpoint Create model output endpoint Apr 15, 2025
Copy link
Contributor

@bschroeter bschroeter left a comment

Choose a reason for hiding this comment

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

A few small things as indicated in the comments, most of them around the semantics of how things are currently named etc. This is mostly about future-proofing what we have and making it easier to enhance the client going forward.

Some other things that would be useful here:

  • Update the docs to include details about how to use the new endpoint(s)
  • More comprehensive tests, beyond just ensuring that a command doesn't return non-zero, but then using other parts of the client to ensure that what was created can indeed be returned as expected.

Nice work.

@abhaasgoyal abhaasgoyal marked this pull request as draft April 15, 2025 23:35
@abhaasgoyal abhaasgoyal requested a review from bschroeter April 16, 2025 02:04
@abhaasgoyal abhaasgoyal requested a review from bschroeter April 22, 2025 02:08
@abhaasgoyal abhaasgoyal marked this pull request as ready for review April 22, 2025 03:28
Copy link
Contributor

@bschroeter bschroeter left a comment

Choose a reason for hiding this comment

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

A few small suggestions, but in the interest of closing this PR we can shunt them to another issue.

Nice work.

@abhaasgoyal abhaasgoyal force-pushed the 3-model-output-endpoint branch from e5e8f5f to 1a200ee Compare April 23, 2025 00:37
@abhaasgoyal abhaasgoyal merged commit c57f6e1 into main Apr 23, 2025
2 checks 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.

3 participants