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

Fix batch range line selector and also allow to create logs on draft commits #202

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

andresgutgon
Copy link
Contributor

@andresgutgon andresgutgon commented Sep 17, 2024

What?

Line picker selector was not working properly
Issue: #180

🐛 Evaluation logs not created on DRAFT commits

While fixing the batch line range selector we saw that evaluation logs where not created when in a draft commit. The reason is that we were enqueueing jobs with document.commitId instead of draft commit id. Now we pass commitUuid that is in the URL of the draft commit.

What?

  • Fix draft commits not creating evaluation logs
  • Fix batch range line selector

@andresgutgon andresgutgon added the 🚧 wip Work in progress label Sep 17, 2024
@andresgutgon andresgutgon force-pushed the fix/batch-rows-range-picker branch from 66ed99a to 4a0ea8d Compare September 17, 2024 16:01
@andresgutgon andresgutgon removed the 🚧 wip Work in progress label Sep 17, 2024
packages/env/src/index.ts Outdated Show resolved Hide resolved
csansoon
csansoon previously approved these changes Sep 17, 2024
options={providerOptions}
value={selectedProvider}
disabled={
disabledMetadataSelectors || isLoading || !providerOptions.length
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

does the placeholder show when disabled? we should tell the user they are using the default provider if none selected

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If non selected it says "default provider" yes

@@ -43,7 +47,7 @@ export const runBatchEvaluationJob = async (
if (!workspace) throw new NotFoundError('Workspace not found')

const commit = await new CommitsRepository(workspace.id)
.find(document.commitId)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't get this, how can document.commitid be different than commitUuid? the documentversion always poins to its commit no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before it was the commitId the document was created (live). Now is the commit that the user is seeing. live or a draft commit. So, the logs are attached to that commit. Which is what we want

@andresgutgon andresgutgon merged commit f721895 into main Sep 18, 2024
3 checks passed
@andresgutgon andresgutgon deleted the fix/batch-rows-range-picker branch September 18, 2024 07:04
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