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

Feature/carecon 792 add Job and CheckJobStatus into go-force #1

Merged
merged 22 commits into from
Aug 13, 2024

Conversation

sBurmester
Copy link

No description provided.

@sBurmester sBurmester changed the title WIP: Feature/carecon 792 add check job status Feature/carecon 792 add Job and CheckJobStatus into go-force Aug 9, 2024
force/job.go Show resolved Hide resolved
@sBurmester sBurmester force-pushed the feature/CARECON-792_add_check_job_status branch from ebf4524 to 1581789 Compare August 12, 2024 12:57
force/job.go Show resolved Hide resolved
force/job.go Outdated
return nil
}

func (job *Job) GetByteCount() int {

Choose a reason for hiding this comment

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

Brauchen wir die hier noch ?

Copy link
Author

Choose a reason for hiding this comment

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

Wird im SCB genutzt. Weiß nicht ob wir das dort brauchen. Ansonsten kann gleich das ganze byte konstrukt aus dem Job verschwinden. Da ich hier nach eh an den SCB ran gehe kann ich das auch ganz entfernen, da es wirklich nur für den byte count genutzt wurd.

Copy link

Choose a reason for hiding this comment

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

bitte rausnehmen :) wird meines Wissens nur geloggt, liefert aber keinen Mehrwert

force/job.go Outdated
}
}

func (job *Job) GetForceApi() *ForceApi { return job.forceApi }

Choose a reason for hiding this comment

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

mMn können die hier raus da dass die Felder sind die wir auch dem create Job mitgeben müssen und der client immer mit default initialisiert wird. Der Ersteller das Jobs hätte immer alle Infos schon da... sehe hier keinen Mehrwert

Copy link
Author

Choose a reason for hiding this comment

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

Im Grunde machen die ganzen GetXyz's da keinen Sinn. Ich nehm die komplett raus

force/job.go Outdated
}

contentUrl := job.info.ContentURL
if contentUrl[0:1] != `/` {
Copy link

Choose a reason for hiding this comment

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

es gibt strings.HasPrefix was deutlich einfacher zu lesen ist :)

force/job.go Outdated
return fmt.Errorf("unexpected StatusCode on PUT batch: %d (%s), %s", res.StatusCode, res.Status, string(errb))
}

statusURI := fmt.Sprintf("/services/data/"+job.apiVersion+"/jobs/ingest/%s", job.info.Id)
Copy link

Choose a reason for hiding this comment

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

Wieso nicht alles als formattierten String, sondern zusätzlich noch String-Concatenation?

Copy link
Author

Choose a reason for hiding this comment

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

Gute Frage, ist beim Umzug so mit rüber gekommen. Ich ändere das! 👍

force/job.go Outdated
return nil
}

func (job *Job) GetByteCount() int {
Copy link

Choose a reason for hiding this comment

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

bitte rausnehmen :) wird meines Wissens nur geloggt, liefert aber keinen Mehrwert

- removes bytes from jobs
- removes GetByteCount-function
- changes function signature for ProgressReporter
- changes check for prefix from check first rune to strings.HasPrefix
@sBurmester sBurmester force-pushed the feature/CARECON-792_add_check_job_status branch from 8229617 to de36bcd Compare August 13, 2024 07:24
@sBurmester sBurmester merged commit 8f491ab into main Aug 13, 2024
14 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