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

Iris: Upgrade settings system for Pyris V2 #9247

Merged
merged 8 commits into from
Oct 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/dev/guidelines/database.rst
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ Best Practices
// IrisSubSettings.java
@Column(name = "allowed_models")
@Convert(converter = IrisModelListConverter.class)
private TreeSet<String> allowedModels = new TreeSet<>();
private TreeSet<String> allowedVariants = new TreeSet<>();
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Improved guidance on collection types and persistence!

The changes to the "Best Practices" section provide valuable insights, particularly the guidance on choosing between Set and List for collections. The explanation of how to handle ordered collections and the note on persisting objects in ordered collections are especially helpful for preventing common errors.

Consider adding a brief explanation of why the procedure for persisting objects in ordered collections is necessary (e.g., to maintain the correct order and avoid null index issues) to help developers understand the reasoning behind this approach.

* **Ordered Collection with duplicates**: When you want to order the collection of (potentially duplicated) objects of the relationship, then always use a ``List``. It is important to note here that there is no inherent order in a database table. One could argue that you can use the ``id`` field for the ordering, but there are edge cases where this can lead to problems. Therefore, for an ordered collection with duplicates, **always** annotate it with ``@OrderColumn``. An order column indicates to Hibernate that we want to order our collection based on a specific column of our data table. By default, the column name it expects is *tablenameS\_order*. For ordered collections, we also recommend that you annotate them with ``cascade = CascadeType.ALL`` and ``orphanRemoval = true``. E.g.:
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

/**
* An IrisSession represents a list of messages of Artemis, a user, and an LLM.
* See {@link IrisExerciseChatSession} and {@link IrisHestiaSession} for concrete implementations.
* See {@link IrisExerciseChatSession} and {@link IrisCourseChatSession} for concrete implementations.
*/
@Entity
@Table(name = "iris_session")
Expand All @@ -40,7 +40,6 @@
@JsonSubTypes({
@JsonSubTypes.Type(value = IrisExerciseChatSession.class, name = "chat"), // TODO: Legacy. Should ideally be "exercise_chat"
@JsonSubTypes.Type(value = IrisCourseChatSession.class, name = "course_chat"),
@JsonSubTypes.Type(value = IrisHestiaSession.class, name = "hestia"),
})
// @formatter:on
@JsonInclude(JsonInclude.Include.NON_EMPTY)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,31 +1,21 @@
package de.tum.cit.aet.artemis.iris.domain.settings;

import jakarta.annotation.Nullable;
import jakarta.persistence.CascadeType;
import jakarta.persistence.Column;
import jakarta.persistence.DiscriminatorValue;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.OneToOne;

import com.fasterxml.jackson.annotation.JsonInclude;

import de.tum.cit.aet.artemis.iris.domain.IrisTemplate;

/**
* An {@link IrisSubSettings} implementation for chat settings.
* Chat settings notably provide settings for the rate limit.
* Chat settings provide a single {@link IrisTemplate} for the chat messages.
*/
@Entity
@DiscriminatorValue("CHAT")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public class IrisChatSubSettings extends IrisSubSettings {

@Nullable
@OneToOne(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER)
private IrisTemplate template;

@Nullable
@Column(name = "rate_limit")
private Integer rateLimit;
Expand All @@ -34,15 +24,6 @@ public class IrisChatSubSettings extends IrisSubSettings {
@Column(name = "rate_limit_timeframe_hours")
private Integer rateLimitTimeframeHours;

@Nullable
public IrisTemplate getTemplate() {
return template;
}

public void setTemplate(@Nullable IrisTemplate template) {
this.template = template;
}

@Nullable
public Integer getRateLimit() {
return rateLimit;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,36 +1,16 @@
package de.tum.cit.aet.artemis.iris.domain.settings;

import jakarta.annotation.Nullable;
import jakarta.persistence.CascadeType;
import jakarta.persistence.DiscriminatorValue;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.OneToOne;

import com.fasterxml.jackson.annotation.JsonInclude;

import de.tum.cit.aet.artemis.iris.domain.IrisTemplate;

/**
* An {@link IrisSubSettings} implementation for the settings for competency generation.
* CompetencyGeneration settings provide a single {@link IrisTemplate}
*/
@Entity
@DiscriminatorValue("COMPETENCY_GENERATION")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public class IrisCompetencyGenerationSubSettings extends IrisSubSettings {

@Nullable
@OneToOne(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER)
private IrisTemplate template;

@Nullable
public IrisTemplate getTemplate() {
return template;
}

public void setTemplate(@Nullable IrisTemplate template) {
this.template = template;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,10 @@ public class IrisCourseSettings extends IrisSettings {
@JoinColumn(name = "iris_lecture_ingestion_settings_id")
private IrisLectureIngestionSubSettings irisLectureIngestionSettings;

@OneToOne(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER)
@JoinColumn(name = "iris_hestia_settings_id")
private IrisHestiaSubSettings irisHestiaSettings;

@OneToOne(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER, optional = false)
@JoinColumn(name = "iris_competency_generation_settings_id")
private IrisCompetencyGenerationSubSettings irisCompetencyGenerationSettings;

@Override
public boolean isValid() {
return course != null;
}

public Course getCourse() {
return course;
}
Expand Down Expand Up @@ -73,16 +64,6 @@ public void setIrisChatSettings(IrisChatSubSettings irisChatSettings) {
this.irisChatSettings = irisChatSettings;
}

@Override
public IrisHestiaSubSettings getIrisHestiaSettings() {
return irisHestiaSettings;
}

@Override
public void setIrisHestiaSettings(IrisHestiaSubSettings irisHestiaSettings) {
this.irisHestiaSettings = irisHestiaSettings;
}

@Override
public IrisCompetencyGenerationSubSettings getIrisCompetencyGenerationSettings() {
return irisCompetencyGenerationSettings;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ public class IrisExerciseSettings extends IrisSettings {
@JoinColumn(name = "iris_chat_settings_id")
private IrisChatSubSettings irisChatSettings;

@Override
public boolean isValid() {
return exercise != null;
}

public Exercise getExercise() {
return exercise;
}
Expand Down Expand Up @@ -60,16 +55,6 @@ public void setIrisChatSettings(IrisChatSubSettings irisChatSettings) {
this.irisChatSettings = irisChatSettings;
}

@Override
public IrisHestiaSubSettings getIrisHestiaSettings() {
return null;
}

@Override
public void setIrisHestiaSettings(IrisHestiaSubSettings irisHestiaSettings) {

}

@Override
public IrisCompetencyGenerationSubSettings getIrisCompetencyGenerationSettings() {
return null;
Expand Down
Loading
Loading